* [PATCH v7 01/16] security: new security_inode_init_security API adds function callback
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 02/16] integrity: move ima inode integrity data management Mimi Zohar
` (17 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Dave Chinner, Steven Whitehouse, Jan Kara, Chris Mason,
Dave Kleikamp, David Woodhouse, Alex Elder, Mark Fasheh,
Joel Becker, Mimi Zohar
This patch changes the security_inode_init_security API by adding a
filesystem specific callback to write security extended attributes.
This change is in preparation for supporting the initialization of
multiple LSM xattrs and the EVM xattr. Initially the callback function
walks an array of xattrs, writing each xattr separately, but could be
optimized to write multiple xattrs at once.
For existing security_inode_init_security() calls, which have not yet
been converted to use the new callback function, such as those in
reiserfs and ocfs2, this patch defines security_old_inode_init_security().
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
fs/btrfs/xattr.c | 52 +++++++++++++++++++-------------------
fs/ext2/xattr_security.c | 34 +++++++++++++-----------
fs/ext3/xattr_security.c | 36 ++++++++++++++-----------
fs/ext4/xattr_security.c | 36 ++++++++++++++-----------
fs/gfs2/inode.c | 38 +++++++++++++--------------
fs/jffs2/security.c | 37 ++++++++++++++------------
fs/jfs/xattr.c | 57 ++++++++++++++++++++---------------------
fs/ocfs2/xattr.c | 38 +++++++++++++++++----------
fs/reiserfs/xattr_security.c | 4 +-
fs/xfs/linux-2.6/xfs_iops.c | 39 ++++++++++++++--------------
include/linux/security.h | 17 +++++++++---
include/linux/xattr.h | 6 ++++
mm/shmem.c | 4 +-
security/security.c | 39 ++++++++++++++++++++++++++--
14 files changed, 252 insertions(+), 185 deletions(-)
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 5366fe4..53c2317 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -360,36 +360,36 @@ int btrfs_removexattr(struct dentry *dentry, const char *name)
XATTR_REPLACE);
}
-int btrfs_xattr_security_init(struct btrfs_trans_handle *trans,
- struct inode *inode, struct inode *dir,
- const struct qstr *qstr)
+int btrfs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
{
- int err;
- size_t len;
- void *value;
- char *suffix;
+ const struct xattr *xattr;
+ struct btrfs_trans_handle *trans = fs_info;
char *name;
-
- err = security_inode_init_security(inode, dir, qstr, &suffix, &value,
- &len);
- if (err) {
- if (err == -EOPNOTSUPP)
- return 0;
- return err;
- }
-
- name = kmalloc(XATTR_SECURITY_PREFIX_LEN + strlen(suffix) + 1,
- GFP_NOFS);
- if (!name) {
- err = -ENOMEM;
- } else {
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
strcpy(name, XATTR_SECURITY_PREFIX);
- strcpy(name + XATTR_SECURITY_PREFIX_LEN, suffix);
- err = __btrfs_setxattr(trans, inode, name, value, len, 0);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+ err = __btrfs_setxattr(trans, inode, name,
+ xattr->value, xattr->value_len, 0);
kfree(name);
+ if (err < 0)
+ break;
}
-
- kfree(suffix);
- kfree(value);
return err;
}
+
+int btrfs_xattr_security_init(struct btrfs_trans_handle *trans,
+ struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &btrfs_initxattrs, trans);
+}
diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index 5d979b4..fc80cf4 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -46,26 +46,28 @@ ext2_xattr_security_set(struct dentry *dentry, const char *name,
value, size, flags);
}
+int ext2_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = ext2_xattr_set(inode, EXT2_XATTR_INDEX_SECURITY,
+ xattr->name, xattr->value,
+ xattr->value_len, 0);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
int
ext2_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr)
{
- int err;
- size_t len;
- void *value;
- char *name;
-
- err = security_inode_init_security(inode, dir, qstr, &name, &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);
- return err;
+ return security_inode_init_security(inode, dir, qstr,
+ &ext2_initxattrs, NULL);
}
const struct xattr_handler ext2_xattr_security_handler = {
diff --git a/fs/ext3/xattr_security.c b/fs/ext3/xattr_security.c
index b8d9f83..4202683 100644
--- a/fs/ext3/xattr_security.c
+++ b/fs/ext3/xattr_security.c
@@ -48,26 +48,30 @@ ext3_xattr_security_set(struct dentry *dentry, const char *name,
name, value, size, flags);
}
+int ext3_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ handle_t *handle = fs_info;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = ext3_xattr_set_handle(handle, inode,
+ EXT3_XATTR_INDEX_SECURITY,
+ xattr->name, xattr->value,
+ xattr->value_len, 0);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
int
ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir,
const struct qstr *qstr)
{
- int err;
- size_t len;
- void *value;
- char *name;
-
- err = security_inode_init_security(inode, dir, qstr, &name, &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);
- return err;
+ return security_inode_init_security(inode, dir, qstr,
+ &ext3_initxattrs, handle);
}
const struct xattr_handler ext3_xattr_security_handler = {
diff --git a/fs/ext4/xattr_security.c b/fs/ext4/xattr_security.c
index 007c3bf..e8ccf3a 100644
--- a/fs/ext4/xattr_security.c
+++ b/fs/ext4/xattr_security.c
@@ -48,26 +48,30 @@ ext4_xattr_security_set(struct dentry *dentry, const char *name,
name, value, size, flags);
}
+int ext4_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ handle_t *handle = fs_info;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = ext4_xattr_set_handle(handle, inode,
+ EXT4_XATTR_INDEX_SECURITY,
+ xattr->name, xattr->value,
+ xattr->value_len, 0);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
int
ext4_init_security(handle_t *handle, struct inode *inode, struct inode *dir,
const struct qstr *qstr)
{
- int err;
- size_t len;
- void *value;
- char *name;
-
- err = security_inode_init_security(inode, dir, qstr, &name, &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);
- return err;
+ return security_inode_init_security(inode, dir, qstr,
+ &ext4_initxattrs, handle);
}
const struct xattr_handler ext4_xattr_security_handler = {
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 03e0c52..b6dabfe 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -624,31 +624,29 @@ fail:
return error;
}
-static int gfs2_security_init(struct gfs2_inode *dip, struct gfs2_inode *ip,
- const struct qstr *qstr)
+int gfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
{
- int err;
- size_t len;
- void *value;
- char *name;
-
- err = security_inode_init_security(&ip->i_inode, &dip->i_inode, qstr,
- &name, &value, &len);
-
- if (err) {
- if (err == -EOPNOTSUPP)
- return 0;
- return err;
+ const struct xattr *xattr;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = __gfs2_xattr_set(inode, xattr->name, xattr->value,
+ xattr->value_len, 0,
+ GFS2_EATYPE_SECURITY);
+ if (err < 0)
+ break;
}
-
- err = __gfs2_xattr_set(&ip->i_inode, name, value, len, 0,
- GFS2_EATYPE_SECURITY);
- kfree(value);
- kfree(name);
-
return err;
}
+static int gfs2_security_init(struct gfs2_inode *dip, struct gfs2_inode *ip,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(&ip->i_inode, &dip->i_inode, qstr,
+ &gfs2_initxattrs, NULL);
+}
+
/**
* gfs2_create_inode - Create a new inode
* @dir: The parent directory
diff --git a/fs/jffs2/security.c b/fs/jffs2/security.c
index cfeb716..2f27244 100644
--- a/fs/jffs2/security.c
+++ b/fs/jffs2/security.c
@@ -22,26 +22,29 @@
#include <linux/security.h>
#include "nodelist.h"
-/* ---- Initial Security Label Attachment -------------- */
-int jffs2_init_security(struct inode *inode, struct inode *dir,
- const struct qstr *qstr)
+/* ---- Initial Security Label(s) Attachment callback --- */
+int jffs2_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
{
- int rc;
- size_t len;
- void *value;
- char *name;
-
- rc = security_inode_init_security(inode, dir, qstr, &name, &value, &len);
- if (rc) {
- if (rc == -EOPNOTSUPP)
- return 0;
- return rc;
+ const struct xattr *xattr;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = do_jffs2_setxattr(inode, JFFS2_XPREFIX_SECURITY,
+ xattr->name, xattr->value,
+ xattr->value_len, 0);
+ if (err < 0)
+ break;
}
- rc = do_jffs2_setxattr(inode, JFFS2_XPREFIX_SECURITY, name, value, len, 0);
+ return err;
+}
- kfree(name);
- kfree(value);
- return rc;
+/* ---- Initial Security Label(s) Attachment ----------- */
+int jffs2_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &jffs2_initxattrs, NULL);
}
/* ---- XATTR Handler for "security.*" ----------------- */
diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
index 24838f1..516afd5 100644
--- a/fs/jfs/xattr.c
+++ b/fs/jfs/xattr.c
@@ -1091,38 +1091,37 @@ int jfs_removexattr(struct dentry *dentry, const char *name)
}
#ifdef CONFIG_JFS_SECURITY
-int jfs_init_security(tid_t tid, struct inode *inode, struct inode *dir,
- const struct qstr *qstr)
+int jfs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
{
- int rc;
- size_t len;
- void *value;
- char *suffix;
+ const struct xattr *xattr;
+ tid_t *tid = fs_info;
char *name;
-
- rc = security_inode_init_security(inode, dir, qstr, &suffix, &value,
- &len);
- if (rc) {
- if (rc == -EOPNOTSUPP)
- return 0;
- return rc;
- }
- name = kmalloc(XATTR_SECURITY_PREFIX_LEN + 1 + strlen(suffix),
- GFP_NOFS);
- if (!name) {
- rc = -ENOMEM;
- goto kmalloc_failed;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+
+ err = __jfs_setxattr(*tid, inode, name,
+ xattr->value, xattr->value_len, 0);
+ kfree(name);
+ if (err < 0)
+ break;
}
- strcpy(name, XATTR_SECURITY_PREFIX);
- strcpy(name + XATTR_SECURITY_PREFIX_LEN, suffix);
-
- rc = __jfs_setxattr(tid, inode, name, value, len, 0);
-
- kfree(name);
-kmalloc_failed:
- kfree(suffix);
- kfree(value);
+ return err;
+}
- return rc;
+int jfs_init_security(tid_t tid, struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &jfs_initxattrs, &tid);
}
#endif
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 81ecf9c..40d9956 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -7185,20 +7185,9 @@ int ocfs2_init_security_and_acl(struct inode *dir,
{
int ret = 0;
struct buffer_head *dir_bh = NULL;
- struct ocfs2_security_xattr_info si = {
- .enable = 1,
- };
- ret = ocfs2_init_security_get(inode, dir, qstr, &si);
+ ret = ocfs2_init_security_get(inode, dir, qstr, NULL);
if (!ret) {
- ret = ocfs2_xattr_set(inode, OCFS2_XATTR_INDEX_SECURITY,
- si.name, si.value, si.value_len,
- XATTR_CREATE);
- if (ret) {
- mlog_errno(ret);
- goto leave;
- }
- } else if (ret != -EOPNOTSUPP) {
mlog_errno(ret);
goto leave;
}
@@ -7255,6 +7244,22 @@ static int ocfs2_xattr_security_set(struct dentry *dentry, const char *name,
name, value, size, flags);
}
+int ocfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = ocfs2_xattr_set(inode, OCFS2_XATTR_INDEX_SECURITY,
+ xattr->name, xattr->value,
+ xattr->value_len, XATTR_CREATE);
+ if (err)
+ break;
+ }
+ return err;
+}
+
int ocfs2_init_security_get(struct inode *inode,
struct inode *dir,
const struct qstr *qstr,
@@ -7263,8 +7268,13 @@ int ocfs2_init_security_get(struct inode *inode,
/* check whether ocfs2 support feature xattr */
if (!ocfs2_supports_xattr(OCFS2_SB(dir->i_sb)))
return -EOPNOTSUPP;
- return security_inode_init_security(inode, dir, qstr, &si->name,
- &si->value, &si->value_len);
+ if (si)
+ return security_old_inode_init_security(inode, dir, qstr,
+ &si->name, &si->value,
+ &si->value_len);
+
+ return security_inode_init_security(inode, dir, qstr,
+ &ocfs2_initxattrs, NULL);
}
int ocfs2_init_security_set(handle_t *handle,
diff --git a/fs/reiserfs/xattr_security.c b/fs/reiserfs/xattr_security.c
index ef66c18..534668f 100644
--- a/fs/reiserfs/xattr_security.c
+++ b/fs/reiserfs/xattr_security.c
@@ -66,8 +66,8 @@ int reiserfs_security_init(struct inode *dir, struct inode *inode,
if (IS_PRIVATE(dir))
return 0;
- error = security_inode_init_security(inode, dir, qstr, &sec->name,
- &sec->value, &sec->length);
+ error = security_old_inode_init_security(inode, dir, qstr, &sec->name,
+ &sec->value, &sec->length);
if (error) {
if (error == -EOPNOTSUPP)
error = 0;
diff --git a/fs/xfs/linux-2.6/xfs_iops.c b/fs/xfs/linux-2.6/xfs_iops.c
index dd21784..1f8eeb8 100644
--- a/fs/xfs/linux-2.6/xfs_iops.c
+++ b/fs/xfs/linux-2.6/xfs_iops.c
@@ -93,37 +93,38 @@ xfs_mark_inode_dirty(
mark_inode_dirty(inode);
}
+
+int xfs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ struct xfs_inode *ip = XFS_I(inode);
+ int error = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ error = xfs_attr_set(ip, xattr->name, xattr->value,
+ xattr->value_len, ATTR_SECURE);
+ if (error < 0)
+ break;
+ }
+ return error;
+}
+
/*
* Hook in SELinux. This is not quite correct yet, what we really need
* here (as we do for default ACLs) is a mechanism by which creation of
* these attrs can be journalled at inode creation time (along with the
* inode, of course, such that log replay can't cause these to be lost).
*/
+
STATIC int
xfs_init_security(
struct inode *inode,
struct inode *dir,
const struct qstr *qstr)
{
- struct xfs_inode *ip = XFS_I(inode);
- size_t length;
- void *value;
- unsigned char *name;
- int error;
-
- error = security_inode_init_security(inode, dir, qstr, (char **)&name,
- &value, &length);
- if (error) {
- if (error == -EOPNOTSUPP)
- return 0;
- return -error;
- }
-
- error = xfs_attr_set(ip, name, value, length, ATTR_SECURE);
-
- kfree(name);
- kfree(value);
- return error;
+ return security_inode_init_security(inode, dir, qstr,
+ &xfs_initxattrs, NULL);
}
static void
diff --git a/include/linux/security.h b/include/linux/security.h
index 8ce59ef..ebad5fb 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -36,6 +36,7 @@
#include <linux/key.h>
#include <linux/xfrm.h>
#include <linux/slab.h>
+#include <linux/xattr.h>
#include <net/flow.h>
/* Maximum number of letters for an LSM name string */
@@ -147,6 +148,10 @@ extern int mmap_min_addr_handler(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp, loff_t *ppos);
#endif
+/* security_inode_init_security callback function to write xattrs */
+typedef int (*initxattrs) (struct inode *inode,
+ const struct xattr *xattr_array, void *fs_data);
+
#ifdef CONFIG_SECURITY
struct security_mnt_opts {
@@ -1704,8 +1709,11 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
int security_inode_alloc(struct inode *inode);
void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
- const struct qstr *qstr, char **name,
- void **value, size_t *len);
+ const struct qstr *qstr,
+ initxattrs initxattrs, void *fs_data);
+int security_old_inode_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr, char **name,
+ void **value, size_t *len);
int security_inode_create(struct inode *dir, struct dentry *dentry, int mode);
int security_inode_link(struct dentry *old_dentry, struct inode *dir,
struct dentry *new_dentry);
@@ -2035,9 +2043,8 @@ static inline void security_inode_free(struct inode *inode)
static inline int security_inode_init_security(struct inode *inode,
struct inode *dir,
const struct qstr *qstr,
- char **name,
- void **value,
- size_t *len)
+ initxattrs initxattrs,
+ void *fs_data)
{
return -EOPNOTSUPP;
}
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index aed54c5..7a37866 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -67,6 +67,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/mm/shmem.c b/mm/shmem.c
index d221a1c..518f27c 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1877,7 +1877,7 @@ shmem_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
inode = shmem_get_inode(dir->i_sb, dir, mode, dev, VM_NORESERVE);
if (inode) {
error = security_inode_init_security(inode, dir,
- &dentry->d_name, NULL,
+ &dentry->d_name,
NULL, NULL);
if (error) {
if (error != -EOPNOTSUPP) {
@@ -2017,7 +2017,7 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
if (!inode)
return -ENOSPC;
- error = security_inode_init_security(inode, dir, &dentry->d_name, NULL,
+ error = security_inode_init_security(inode, dir, &dentry->d_name,
NULL, NULL);
if (error) {
if (error != -EOPNOTSUPP) {
diff --git a/security/security.c b/security/security.c
index 4ba6d4c..3464d58 100644
--- a/security/security.c
+++ b/security/security.c
@@ -18,6 +18,8 @@
#include <linux/security.h>
#include <linux/ima.h>
+#define MAX_LSM_XATTR 1
+
/* Boot-time LSM user choice */
static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
CONFIG_DEFAULT_SECURITY;
@@ -339,15 +341,46 @@ void security_inode_free(struct inode *inode)
}
int security_inode_init_security(struct inode *inode, struct inode *dir,
- const struct qstr *qstr, char **name,
- void **value, size_t *len)
+ const struct qstr *qstr,
+ const initxattrs initxattrs, void *fs_data)
+{
+ struct xattr new_xattrs[MAX_LSM_XATTR + 1];
+ struct xattr *lsm_xattr;
+ int ret;
+
+ if (unlikely(IS_PRIVATE(inode)))
+ return -EOPNOTSUPP;
+
+ memset(new_xattrs, 0, sizeof new_xattrs);
+ if (!initxattrs)
+ return security_ops->inode_init_security(inode, dir, qstr,
+ NULL, NULL, NULL);
+ lsm_xattr = new_xattrs;
+ ret = security_ops->inode_init_security(inode, dir, qstr,
+ &lsm_xattr->name,
+ &lsm_xattr->value,
+ &lsm_xattr->value_len);
+ if (ret)
+ goto out;
+ ret = initxattrs(inode, new_xattrs, fs_data);
+out:
+ kfree(lsm_xattr->name);
+ kfree(lsm_xattr->value);
+
+ return (ret == -EOPNOTSUPP) ? 0 : ret;
+}
+EXPORT_SYMBOL(security_inode_init_security);
+
+int security_old_inode_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr, char **name,
+ void **value, size_t *len)
{
if (unlikely(IS_PRIVATE(inode)))
return -EOPNOTSUPP;
return security_ops->inode_init_security(inode, dir, qstr, name, value,
len);
}
-EXPORT_SYMBOL(security_inode_init_security);
+EXPORT_SYMBOL(security_old_inode_init_security);
#ifdef CONFIG_SECURITY_PATH
int security_path_mknod(struct path *dir, struct dentry *dentry, int mode,
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 02/16] integrity: move ima inode integrity data management
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 01/16] security: new security_inode_init_security API adds function callback Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 03/16] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp Mimi Zohar
` (16 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
Move the inode integrity data(iint) management up to the integrity directory
in order to share the iint among the different integrity models.
Changelog:
- don't define MAX_DIGEST_SIZE
- rename several globally visible 'ima_' prefixed functions, structs,
locks, etc to 'integrity_'
- replace '20' with SHA1_DIGEST_SIZE
- reflect location change in appropriate Kconfig and Makefiles
- remove unnecessary initialization of iint_initialized to 0
- rebased on current ima_iint.c
- define integrity_iint_store/lock as static
There should be no other functional changes.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---
include/linux/ima.h | 13 ---
include/linux/integrity.h | 30 +++++++
security/Kconfig | 2 +-
security/Makefile | 4 +-
security/integrity/Kconfig | 6 ++
security/integrity/Makefile | 10 ++
security/integrity/iint.c | 170 +++++++++++++++++++++++++++++++++++++
security/integrity/ima/Kconfig | 1 +
security/integrity/ima/Makefile | 2 +-
security/integrity/ima/ima.h | 29 ++-----
security/integrity/ima/ima_api.c | 7 +-
security/integrity/ima/ima_iint.c | 169 ------------------------------------
security/integrity/ima/ima_main.c | 12 ++--
security/integrity/integrity.h | 35 ++++++++
security/security.c | 3 +-
15 files changed, 277 insertions(+), 216 deletions(-)
create mode 100644 include/linux/integrity.h
create mode 100644 security/integrity/Kconfig
create mode 100644 security/integrity/Makefile
create mode 100644 security/integrity/iint.c
delete mode 100644 security/integrity/ima/ima_iint.c
create mode 100644 security/integrity/integrity.h
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 09e6e62..6ac8e50 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);
@@ -27,16 +25,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;
@@ -51,6 +39,5 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
{
return 0;
}
-
#endif /* CONFIG_IMA_H */
#endif /* _LINUX_IMA_H */
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
new file mode 100644
index 0000000..9059812
--- /dev/null
+++ b/include/linux/integrity.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2009 IBM Corporation
+ * Author: Mimi Zohar <zohar@us.ibm.com>
+ *
+ * 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
+
+#include <linux/fs.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 e0f08b5..22847a8 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -186,7 +186,7 @@ source security/smack/Kconfig
source security/tomoyo/Kconfig
source security/apparmor/Kconfig
-source security/integrity/ima/Kconfig
+source security/integrity/Kconfig
choice
prompt "Default security module"
diff --git a/security/Makefile b/security/Makefile
index 8bb0fe9..a5e502f 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -24,5 +24,5 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/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..d17de48
--- /dev/null
+++ b/security/integrity/iint.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2008 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@us.ibm.com>
+ *
+ * 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_iint.c
+ * - implements the integrity hooks: integrity_inode_alloc,
+ * integrity_inode_free
+ * - cache integrity information associated with an inode
+ * using a rbtree tree.
+ */
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/rbtree.h>
+#include "integrity.h"
+
+static struct rb_root integrity_iint_tree = RB_ROOT;
+static DEFINE_SPINLOCK(integrity_iint_lock);
+static struct kmem_cache *iint_cache __read_mostly;
+
+int iint_initialized;
+
+/*
+ * __integrity_iint_find - return the iint associated with an inode
+ */
+static struct integrity_iint_cache *__integrity_iint_find(struct inode *inode)
+{
+ struct integrity_iint_cache *iint;
+ struct rb_node *n = integrity_iint_tree.rb_node;
+
+ assert_spin_locked(&integrity_iint_lock);
+
+ while (n) {
+ iint = rb_entry(n, struct integrity_iint_cache, rb_node);
+
+ if (inode < iint->inode)
+ n = n->rb_left;
+ else if (inode > iint->inode)
+ n = n->rb_right;
+ else
+ break;
+ }
+ if (!n)
+ return NULL;
+
+ return iint;
+}
+
+/*
+ * integrity_iint_find - return the iint associated with an inode
+ */
+struct integrity_iint_cache *integrity_iint_find(struct inode *inode)
+{
+ struct integrity_iint_cache *iint;
+
+ if (!IS_IMA(inode))
+ return NULL;
+
+ spin_lock(&integrity_iint_lock);
+ iint = __integrity_iint_find(inode);
+ spin_unlock(&integrity_iint_lock);
+
+ return iint;
+}
+
+static void iint_free(struct integrity_iint_cache *iint)
+{
+ iint->version = 0;
+ iint->flags = 0UL;
+ kmem_cache_free(iint_cache, iint);
+}
+
+/**
+ * integrity_inode_alloc - allocate an iint associated with an inode
+ * @inode: pointer to the inode
+ */
+int integrity_inode_alloc(struct inode *inode)
+{
+ struct rb_node **p;
+ struct rb_node *new_node, *parent = NULL;
+ struct integrity_iint_cache *new_iint, *test_iint;
+ int rc;
+
+ new_iint = kmem_cache_alloc(iint_cache, GFP_NOFS);
+ if (!new_iint)
+ return -ENOMEM;
+
+ new_iint->inode = inode;
+ new_node = &new_iint->rb_node;
+
+ mutex_lock(&inode->i_mutex); /* i_flags */
+ spin_lock(&integrity_iint_lock);
+
+ p = &integrity_iint_tree.rb_node;
+ while (*p) {
+ parent = *p;
+ test_iint = rb_entry(parent, struct integrity_iint_cache,
+ rb_node);
+ rc = -EEXIST;
+ if (inode < test_iint->inode)
+ p = &(*p)->rb_left;
+ else if (inode > test_iint->inode)
+ p = &(*p)->rb_right;
+ else
+ goto out_err;
+ }
+
+ inode->i_flags |= S_IMA;
+ rb_link_node(new_node, parent, p);
+ rb_insert_color(new_node, &integrity_iint_tree);
+
+ spin_unlock(&integrity_iint_lock);
+ mutex_unlock(&inode->i_mutex); /* i_flags */
+
+ return 0;
+out_err:
+ spin_unlock(&integrity_iint_lock);
+ mutex_unlock(&inode->i_mutex); /* i_flags */
+ iint_free(new_iint);
+
+ return rc;
+}
+
+/**
+ * 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;
+
+ if (!IS_IMA(inode))
+ return;
+
+ spin_lock(&integrity_iint_lock);
+ iint = __integrity_iint_find(inode);
+ rb_erase(&iint->rb_node, &integrity_iint_tree);
+ spin_unlock(&integrity_iint_lock);
+
+ iint_free(iint);
+}
+
+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);
+}
+
+static int __init integrity_iintcache_init(void)
+{
+ iint_cache =
+ kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
+ 0, SLAB_PANIC, init_once);
+ iint_initialized = 1;
+ return 0;
+}
+security_initcall(integrity_iintcache_init);
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index b6ecfd4..19c053b 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -3,6 +3,7 @@
config IMA
bool "Integrity Measurement Architecture(IMA)"
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 08408bd..29d97af 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
@@ -96,34 +98,21 @@ static inline unsigned long ima_hash_key(u8 *digest)
return hash_long(*digest, IMA_HASH_BITS);
}
-/* iint cache flags */
-#define IMA_MEASURED 0x01
-
-/* integrity data associated with an inode */
-struct ima_iint_cache {
- struct rb_node rb_node; /* rooted in ima_iint_tree */
- struct inode *inode; /* back pointer to inode in question */
- u64 version; /* track inode changes */
- unsigned char flags;
- u8 digest[IMA_DIGEST_SIZE];
- struct mutex mutex; /* protects: version, flags, digest */
-};
-
/* LIM API function definitions */
int ima_must_measure(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);
/* rbtree 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(struct inode *inode);
+struct integrity_iint_cache *integrity_iint_insert(struct inode *inode);
+struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
/* IMA policy related functions */
enum ima_hooks { FILE_CHECK = 1, FILE_MMAP, BPRM_CHECK };
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index da36d2c..0d50df0 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -126,7 +126,8 @@ int ima_must_measure(struct inode *inode, int mask, int function)
*
* 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;
@@ -156,8 +157,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 4ae7304..0000000
--- a/security/integrity/ima/ima_iint.c
+++ /dev/null
@@ -1,169 +0,0 @@
-/*
- * Copyright (C) 2008 IBM Corporation
- *
- * Authors:
- * Mimi Zohar <zohar@us.ibm.com>
- *
- * 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 rbtree tree.
- */
-#include <linux/slab.h>
-#include <linux/module.h>
-#include <linux/spinlock.h>
-#include <linux/rbtree.h>
-#include "ima.h"
-
-static struct rb_root ima_iint_tree = RB_ROOT;
-static DEFINE_SPINLOCK(ima_iint_lock);
-static struct kmem_cache *iint_cache __read_mostly;
-
-int iint_initialized = 0;
-
-/*
- * __ima_iint_find - return the iint associated with an inode
- */
-static struct ima_iint_cache *__ima_iint_find(struct inode *inode)
-{
- struct ima_iint_cache *iint;
- struct rb_node *n = ima_iint_tree.rb_node;
-
- assert_spin_locked(&ima_iint_lock);
-
- while (n) {
- iint = rb_entry(n, struct ima_iint_cache, rb_node);
-
- if (inode < iint->inode)
- n = n->rb_left;
- else if (inode > iint->inode)
- n = n->rb_right;
- else
- break;
- }
- if (!n)
- return NULL;
-
- return iint;
-}
-
-/*
- * ima_iint_find - return the iint associated with an inode
- */
-struct ima_iint_cache *ima_iint_find(struct inode *inode)
-{
- struct ima_iint_cache *iint;
-
- if (!IS_IMA(inode))
- return NULL;
-
- spin_lock(&ima_iint_lock);
- iint = __ima_iint_find(inode);
- spin_unlock(&ima_iint_lock);
-
- return iint;
-}
-
-static void iint_free(struct ima_iint_cache *iint)
-{
- iint->version = 0;
- iint->flags = 0UL;
- kmem_cache_free(iint_cache, iint);
-}
-
-/**
- * ima_inode_alloc - allocate an iint associated with an inode
- * @inode: pointer to the inode
- */
-int ima_inode_alloc(struct inode *inode)
-{
- struct rb_node **p;
- struct rb_node *new_node, *parent = NULL;
- struct ima_iint_cache *new_iint, *test_iint;
- int rc;
-
- new_iint = kmem_cache_alloc(iint_cache, GFP_NOFS);
- if (!new_iint)
- return -ENOMEM;
-
- new_iint->inode = inode;
- new_node = &new_iint->rb_node;
-
- mutex_lock(&inode->i_mutex); /* i_flags */
- spin_lock(&ima_iint_lock);
-
- p = &ima_iint_tree.rb_node;
- while (*p) {
- parent = *p;
- test_iint = rb_entry(parent, struct ima_iint_cache, rb_node);
-
- rc = -EEXIST;
- if (inode < test_iint->inode)
- p = &(*p)->rb_left;
- else if (inode > test_iint->inode)
- p = &(*p)->rb_right;
- else
- goto out_err;
- }
-
- inode->i_flags |= S_IMA;
- rb_link_node(new_node, parent, p);
- rb_insert_color(new_node, &ima_iint_tree);
-
- spin_unlock(&ima_iint_lock);
- mutex_unlock(&inode->i_mutex); /* i_flags */
-
- return 0;
-out_err:
- spin_unlock(&ima_iint_lock);
- mutex_unlock(&inode->i_mutex); /* i_flags */
- iint_free(new_iint);
-
- return rc;
-}
-
-/**
- * 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;
-
- if (!IS_IMA(inode))
- return;
-
- spin_lock(&ima_iint_lock);
- iint = __ima_iint_find(inode);
- rb_erase(&iint->rb_node, &ima_iint_tree);
- spin_unlock(&ima_iint_lock);
-
- iint_free(iint);
-}
-
-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);
-}
-
-static int __init ima_iintcache_init(void)
-{
- iint_cache =
- kmem_cache_create("iint_cache", sizeof(struct ima_iint_cache), 0,
- SLAB_PANIC, init_once);
- iint_initialized = 1;
- return 0;
-}
-security_initcall(ima_iintcache_init);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 39d66dc..25f9fe7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -82,7 +82,7 @@ out:
"open_writers");
}
-static void ima_check_last_writer(struct ima_iint_cache *iint,
+static void ima_check_last_writer(struct integrity_iint_cache *iint,
struct inode *inode,
struct file *file)
{
@@ -105,12 +105,12 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
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 (!iint_initialized || !S_ISREG(inode->i_mode))
return;
- iint = ima_iint_find(inode);
+ iint = integrity_iint_find(inode);
if (!iint)
return;
@@ -121,7 +121,7 @@ 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 = 0;
if (!ima_initialized || !S_ISREG(inode->i_mode))
@@ -131,9 +131,9 @@ static int process_measurement(struct file *file, const unsigned char *filename,
if (rc != 0)
return rc;
retry:
- iint = ima_iint_find(inode);
+ iint = integrity_iint_find(inode);
if (!iint) {
- rc = ima_inode_alloc(inode);
+ rc = integrity_inode_alloc(inode);
if (!rc || rc == -EEXIST)
goto retry;
return rc;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
new file mode 100644
index 0000000..7351836
--- /dev/null
+++ b/security/integrity/integrity.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2009-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@us.ibm.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/integrity.h>
+#include <crypto/sha.h>
+
+/* iint cache flags */
+#define IMA_MEASURED 0x01
+
+/* integrity data associated with an inode */
+struct integrity_iint_cache {
+ struct rb_node rb_node; /* rooted in integrity_iint_tree */
+ struct inode *inode; /* back pointer to inode in question */
+ u64 version; /* track inode changes */
+ unsigned char flags;
+ u8 digest[SHA1_DIGEST_SIZE];
+ struct mutex mutex; /* protects: version, flags, digest */
+};
+
+/* rbtree tree calls to lookup, insert, delete
+ * integrity data associated with an inode.
+ */
+struct integrity_iint_cache *integrity_iint_insert(struct inode *inode);
+struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
diff --git a/security/security.c b/security/security.c
index 3464d58..947fdcf 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>
#define MAX_LSM_XATTR 1
@@ -336,7 +337,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.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 03/16] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 01/16] security: new security_inode_init_security API adds function callback Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 02/16] integrity: move ima inode integrity data management Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 04/16] evm: re-release Mimi Zohar
` (15 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
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 <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---
fs/xattr.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/xattr.h | 5 +++-
2 files changed, 62 insertions(+), 1 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index f060663..851808c 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -166,6 +166,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 7a37866..4703f6b 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -84,7 +84,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.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 04/16] evm: re-release
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (2 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 03/16] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 05/16] evm: add support for different security.evm data types Mimi Zohar
` (14 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
EVM protects a file's security extended attributes(xattrs) against integrity
attacks. This patchset provides the framework and an initial method. The
initial method maintains an HMAC-sha1 value across the security extended
attributes, storing the HMAC value as the extended attribute 'security.evm'.
Other methods of validating the integrity of a file's metadata will be posted
separately (eg. EVM-digital-signatures).
While this patchset does authenticate the security xattrs, and
cryptographically binds them to the inode, coming extensions will bind other
directory and inode metadata for more complete protection. To help simplify
the review and upstreaming process, each extension will be posted separately
(eg. IMA-appraisal, IMA-appraisal-directory). For a general overview of the
proposed Linux integrity subsystem, refer to Dave Safford's whitepaper:
http://downloads.sf.net/project/linux-ima/linux-ima/Integrity_overview.pdf.
EVM depends on the Kernel Key Retention System to provide it with a
trusted/encrypted key for the HMAC-sha1 operation. The key is loaded onto the
root's keyring using keyctl. Until EVM receives notification that the key has
been successfully loaded onto the keyring (echo 1 > <securityfs>/evm), EVM can
not create or validate the 'security.evm' xattr, but returns INTEGRITY_UNKNOWN.
Loading the key and signaling EVM should be done as early as possible. Normally
this is done in the initramfs, which has already been measured as part of the
trusted boot. For more information on creating and loading existing
trusted/encrypted keys, refer to Documentation/keys-trusted-encrypted.txt. A
sample dracut patch, which loads the trusted/encrypted key and enables EVM, is
available from http://linux-ima.sourceforge.net/#EVM.
Based on the LSMs enabled, the set of EVM protected security xattrs is defined
at compile. 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 a security
xattr, EVM exports evm_verifyxattr().
Changelog v7:
- Fixed URL in EVM ABI documentation
Changelog v6: (based on Serge Hallyn's review)
- fix URL in patch description
- remove evm_hmac_size definition
- use SHA1_DIGEST_SIZE (removed both MAX_DIGEST_SIZE and evm_hmac_size)
- moved linux include before other includes
- test for crypto_hash_setkey failure
- fail earlier for invalid key
- clear entire encrypted key, even on failure
- check xattr name length before comparing xattr names
Changelog:
- locking based on i_mutex, remove evm_mutex
- using trusted/encrypted keys for storing the EVM key used in the HMAC-sha1
operation.
- replaced crypto hash with shash (Dmitry Kasatkin)
- support for additional methods of verifying the security xattrs
(Dmitry Kasatkin)
- iint not allocated for all regular files, but only for those appraised
- Use cap_sys_admin in lieu of cap_mac_admin
- Use __vfs_setxattr_noperm(), without permission checks, from EVM
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
---
Documentation/ABI/testing/evm | 23 +++
include/linux/integrity.h | 7 +
include/linux/xattr.h | 3 +
security/integrity/Kconfig | 3 +-
security/integrity/Makefile | 2 +
security/integrity/evm/Kconfig | 12 ++
security/integrity/evm/Makefile | 6 +
security/integrity/evm/evm.h | 33 ++++
security/integrity/evm/evm_crypto.c | 183 ++++++++++++++++++++++
security/integrity/evm/evm_main.c | 284 +++++++++++++++++++++++++++++++++++
security/integrity/evm/evm_secfs.c | 108 +++++++++++++
security/integrity/iint.c | 1 +
security/integrity/integrity.h | 1 +
13 files changed, 665 insertions(+), 1 deletions(-)
create mode 100644 Documentation/ABI/testing/evm
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
diff --git a/Documentation/ABI/testing/evm b/Documentation/ABI/testing/evm
new file mode 100644
index 0000000..8374d45
--- /dev/null
+++ b/Documentation/ABI/testing/evm
@@ -0,0 +1,23 @@
+What: security/evm
+Date: March 2011
+Contact: Mimi Zohar <zohar@us.ibm.com>
+Description:
+ EVM protects a file's security extended attributes(xattrs)
+ against integrity attacks. The initial method maintains an
+ HMAC-sha1 value across the extended attributes, storing the
+ value as the extended attribute 'security.evm'.
+
+ EVM depends on the Kernel Key Retention System to provide it
+ with a trusted/encrypted key for the HMAC-sha1 operation.
+ The key is loaded onto the root's keyring using keyctl. Until
+ EVM receives notification that the key has been successfully
+ loaded onto the keyring (echo 1 > <securityfs>/evm), EVM
+ can not create or validate the 'security.evm' xattr, but
+ returns INTEGRITY_UNKNOWN. Loading the key and signaling EVM
+ should be done as early as possible. Normally this is done
+ in the initramfs, which has already been measured as part
+ of the trusted boot. For more information on creating and
+ loading existing trusted/encrypted keys, refer to:
+ Documentation/keys-trusted-encrypted.txt. (A sample dracut
+ patch, which loads the trusted/encrypted key and enables
+ EVM, is available from http://linux-ima.sourceforge.net/#EVM.)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 9059812..e715a2a 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -12,6 +12,13 @@
#include <linux/fs.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 4703f6b..b20cb96 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -30,6 +30,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..73f6540
--- /dev/null
+++ b/security/integrity/evm/Kconfig
@@ -0,0 +1,12 @@
+config EVM
+ boolean "EVM support"
+ depends on SECURITY && KEYS && ENCRYPTED_KEYS
+ select CRYPTO_HMAC
+ select CRYPTO_MD5
+ select CRYPTO_SHA1
+ default n
+ help
+ EVM protects a file's security extended attributes against
+ integrity attacks.
+
+ 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..0787d26
--- /dev/null
+++ b/security/integrity/evm/Makefile
@@ -0,0 +1,6 @@
+#
+# 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..375dc3e
--- /dev/null
+++ b/security/integrity/evm/evm.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2005-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@us.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * 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;
+
+/* List of EVM protected security xattrs */
+extern char *evm_config_xattrnames[];
+
+extern int evm_init_key(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);
+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..d49bb00
--- /dev/null
+++ b/security/integrity/evm/evm_crypto.c
@@ -0,0 +1,183 @@
+/*
+ * Copyright (C) 2005-2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@us.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * 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 <linux/scatterlist.h>
+#include <keys/encrypted-type.h>
+#include "evm.h"
+
+#define EVMKEY "evm-key"
+#define MAX_KEY_SIZE 128
+static unsigned char evmkey[MAX_KEY_SIZE];
+static int evmkey_len = MAX_KEY_SIZE;
+
+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;
+ rc = crypto_hash_setkey(desc->tfm, evmkey, evmkey_len);
+ if (rc)
+ goto out;
+ rc = crypto_hash_init(desc);
+out:
+ 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
+ *
+ * Expects to be called with i_mutex locked.
+ */
+int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
+ const char *xattr_value, size_t xattr_value_len)
+{
+ struct inode *inode = dentry->d_inode;
+ u8 hmac[SHA1_DIGEST_SIZE];
+ int rc = 0;
+
+ rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
+ xattr_value_len, hmac);
+ if (rc == 0)
+ rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
+ hmac, SHA1_DIGEST_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_key(void)
+{
+ struct key *evm_key;
+ struct encrypted_key_payload *ekp;
+ int rc = 0;
+
+ evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
+ if (IS_ERR(evm_key))
+ return -ENOENT;
+
+ down_read(&evm_key->sem);
+ ekp = evm_key->payload.data;
+ if (ekp->decrypted_datalen > MAX_KEY_SIZE) {
+ rc = -EINVAL;
+ goto out;
+ }
+ memcpy(evmkey, ekp->decrypted_data, ekp->decrypted_datalen);
+out:
+ /* burn the original key contents */
+ memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
+ up_read(&evm_key->sem);
+ key_put(evm_key);
+ return rc;
+}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
new file mode 100644
index 0000000..a8fa45f
--- /dev/null
+++ b/security/integrity/evm/evm_main.c
@@ -0,0 +1,284 @@
+/*
+ * Copyright (C) 2005-2010 IBM Corporation
+ *
+ * Author:
+ * Mimi Zohar <zohar@us.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * 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)";
+
+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[SHA1_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 namelen;
+ int found = 0;
+
+ namelen = strlen(req_xattr_name);
+ for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
+ if ((strlen(*xattrname) == namelen)
+ && (strncmp(req_xattr_name, *xattrname, namelen) == 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.
+ *
+ * This function requires the caller to lock the inode's i_mutex before it
+ * is executed.
+ */
+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(inode);
+ if (!iint)
+ return INTEGRITY_UNKNOWN;
+ status = evm_verify_hmac(dentry, xattr_name, xattr_value,
+ xattr_value_len, iint);
+ 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 (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
+ if (!capable(CAP_SYS_ADMIN))
+ 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);
+}
+
+/**
+ * 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.
+ *
+ * No need to take the i_mutex lock here, as this function is called from
+ * __vfs_setxattr_noperm(). The caller of which has taken the inode's
+ * i_mutex lock.
+ */
+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_update_evmxattr(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)
+{
+ struct inode *inode = dentry->d_inode;
+
+ if (!evm_initialized || !evm_protected_xattr(xattr_name))
+ return;
+
+ mutex_lock(&inode->i_mutex);
+ evm_update_evmxattr(dentry, xattr_name, NULL, 0);
+ mutex_unlock(&inode->i_mutex);
+ 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.
+ *
+ * This function is called from notify_change(), which expects the caller
+ * to lock the inode's i_mutex.
+ */
+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_update_evmxattr(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..ac76299
--- /dev/null
+++ b/security/integrity/evm/evm_secfs.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2010 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@us.ibm.com>
+ *
+ * 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_SYS_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_key();
+ if (!error) {
+ evm_initialized = 1;
+ pr_info("EVM: initialized\n");
+ } else
+ pr_err("EVM: initialization failed\n");
+ 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 d17de48..991df20 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -157,6 +157,7 @@ static void init_once(void *foo)
iint->version = 0;
iint->flags = 0UL;
mutex_init(&iint->mutex);
+ iint->hmac_status = INTEGRITY_UNKNOWN;
}
static int __init integrity_iintcache_init(void)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 7351836..397a46b 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -26,6 +26,7 @@ struct integrity_iint_cache {
unsigned char flags;
u8 digest[SHA1_DIGEST_SIZE];
struct mutex mutex; /* protects: version, flags, digest */
+ enum integrity_status hmac_status;
};
/* rbtree tree calls to lookup, insert, delete
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 05/16] evm: add support for different security.evm data types
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (3 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 04/16] evm: re-release Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 06/16] security: imbed evm calls in security hooks Mimi Zohar
` (13 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Dmitry Kasatkin, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
From: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
EVM protects a file's security extended attributes(xattrs) against integrity
attacks. The current patchset maintains an HMAC-sha1 value across the security
xattrs, storing the value as the extended attribute 'security.evm'. We
anticipate other methods for protecting the security extended attributes.
This patch reserves the first byte of 'security.evm' as a place holder for
the type of method.
Changelog v6:
- move evm_ima_xattr_type definition to security/integrity/integrity.h
- defined a structure for the EVM xattr called evm_ima_xattr_data
(based on Serge Hallyn's suggestion)
- removed unnecessary memset
Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
---
include/linux/integrity.h | 1 +
security/integrity/evm/evm_crypto.c | 11 +++++++----
security/integrity/evm/evm_main.c | 10 +++++-----
security/integrity/integrity.h | 11 +++++++++++
4 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index e715a2a..9684433 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -19,6 +19,7 @@ enum integrity_status {
INTEGRITY_UNKNOWN,
};
+/* List of EVM protected security xattrs */
#ifdef CONFIG_INTEGRITY
extern int integrity_inode_alloc(struct inode *inode);
extern void integrity_inode_free(struct inode *inode);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index d49bb00..c631b99 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -141,14 +141,17 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
const char *xattr_value, size_t xattr_value_len)
{
struct inode *inode = dentry->d_inode;
- u8 hmac[SHA1_DIGEST_SIZE];
+ struct evm_ima_xattr_data xattr_data;
int rc = 0;
rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
- xattr_value_len, hmac);
- if (rc == 0)
+ xattr_value_len, xattr_data.digest);
+ if (rc == 0) {
+ xattr_data.type = EVM_XATTR_HMAC;
rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
- hmac, SHA1_DIGEST_SIZE, 0);
+ &xattr_data,
+ sizeof(xattr_data), 0);
+ }
else if (rc == -ENODATA)
rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM);
return rc;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index a8fa45f..c0580dd1 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -51,20 +51,20 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
size_t xattr_value_len,
struct integrity_iint_cache *iint)
{
- char hmac_val[SHA1_DIGEST_SIZE];
+ struct evm_ima_xattr_data xattr_data;
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);
+ xattr_value_len, xattr_data.digest);
if (rc < 0)
return INTEGRITY_UNKNOWN;
- rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, hmac_val, sizeof hmac_val,
- GFP_NOFS);
+ xattr_data.type = EVM_XATTR_HMAC;
+ rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, (u8 *)&xattr_data,
+ sizeof xattr_data, GFP_NOFS);
if (rc < 0)
goto err_out;
iint->hmac_status = INTEGRITY_PASS;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 397a46b..7efbf56 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -18,6 +18,17 @@
/* iint cache flags */
#define IMA_MEASURED 0x01
+enum evm_ima_xattr_type {
+ IMA_XATTR_DIGEST = 0x01,
+ EVM_XATTR_HMAC,
+ EVM_IMA_XATTR_DIGSIG,
+};
+
+struct evm_ima_xattr_data {
+ u8 type;
+ u8 digest[SHA1_DIGEST_SIZE];
+} __attribute__((packed));
+
/* integrity data associated with an inode */
struct integrity_iint_cache {
struct rb_node rb_node; /* rooted in integrity_iint_tree */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 06/16] security: imbed evm calls in security hooks
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (4 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 05/16] evm: add support for different security.evm data types Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 07/16] evm: evm_inode_post_removexattr Mimi Zohar
` (12 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
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.)
Changelog:
- Don't define evm_verifyxattr(), unless CONFIG_INTEGRITY is enabled.
- xattr_name is a 'const', value is 'void *'
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---
include/linux/evm.h | 56 +++++++++++++++++++++++++++++++++++++
security/integrity/evm/evm_main.c | 1 +
security/security.c | 16 +++++++++-
3 files changed, 71 insertions(+), 2 deletions(-)
create mode 100644 include/linux/evm.h
diff --git a/include/linux/evm.h b/include/linux/evm.h
new file mode 100644
index 0000000..8b4e9e3
--- /dev/null
+++ b/include/linux/evm.h
@@ -0,0 +1,56 @@
+/*
+ * evm.h
+ *
+ * Copyright (c) 2009 IBM Corporation
+ * Author: Mimi Zohar <zohar@us.ibm.com>
+ */
+
+#ifndef _LINUX_EVM_H
+#define _LINUX_EVM_H
+
+#include <linux/integrity.h>
+
+#ifdef CONFIG_EVM
+extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
+ const char *xattr_name,
+ void *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
+#ifdef CONFIG_INTEGRITY
+static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
+ const char *xattr_name,
+ void *xattr_value,
+ size_t xattr_value_len)
+{
+ return INTEGRITY_UNKNOWN;
+}
+#endif
+
+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/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index c0580dd1..1746c36 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -18,6 +18,7 @@
#include <linux/crypto.h>
#include <linux/xattr.h>
#include <linux/integrity.h>
+#include <linux/evm.h>
#include "evm.h"
int evm_initialized;
diff --git a/security/security.c b/security/security.c
index 947fdcf..21a79b3 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>
#define MAX_LSM_XATTR 1
@@ -580,9 +581,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,
@@ -591,6 +597,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)
@@ -609,9 +616,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.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 07/16] evm: evm_inode_post_removexattr
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (5 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 06/16] security: imbed evm calls in security hooks Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 08/16] evm: imbed evm_inode_post_setattr Mimi Zohar
` (11 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
When an EVM protected extended attribute is removed, update 'security.evm'.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---
fs/xattr.c | 5 ++++-
include/linux/evm.h | 9 +++++++++
2 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index 851808c..67583de 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>
@@ -301,8 +302,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 8b4e9e3..a730782 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
#ifdef CONFIG_INTEGRITY
static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
@@ -52,5 +54,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.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 08/16] evm: imbed evm_inode_post_setattr
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (6 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 07/16] evm: evm_inode_post_removexattr Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 09/16] evm: add evm_inode_init_security to initialize new files Mimi Zohar
` (10 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
Changing the inode's metadata may require the 'security.evm' extended
attribute to be re-calculated and updated.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---
fs/attr.c | 5 ++++-
include/linux/evm.h | 6 ++++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/fs/attr.c b/fs/attr.c
index caf2aa5..5ad45d3 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>
/**
* inode_change_ok - check if attribute changes to an inode are allowed
@@ -243,8 +244,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 a730782..33a9247 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -15,6 +15,7 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
const char *xattr_name,
void *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,
@@ -35,6 +36,11 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
}
#endif
+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.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 09/16] evm: add evm_inode_init_security to initialize new files
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (7 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 08/16] evm: imbed evm_inode_post_setattr Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 10/16] evm: call evm_inode_init_security from security_inode_init_security Mimi Zohar
` (9 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
Initialize 'security.evm' for new files.
Changelog v7:
- renamed evm_inode_post_init_security to evm_inode_init_security
- moved struct xattr definition to earlier patch
- allocate xattr name
Changelog v6:
- Use 'struct evm_ima_xattr_data'
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
include/linux/evm.h | 11 ++++++++++
security/integrity/evm/evm.h | 3 ++
security/integrity/evm/evm_crypto.c | 20 ++++++++++++++++++
security/integrity/evm/evm_main.c | 38 +++++++++++++++++++++++++++++++++++
4 files changed, 72 insertions(+), 0 deletions(-)
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 33a9247..7c10761 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_init_security(struct inode *inode,
+ const struct xattr *xattr_array,
+ struct xattr *evm);
#else
#ifdef CONFIG_INTEGRITY
static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
@@ -67,5 +71,12 @@ static inline void evm_inode_post_removexattr(struct dentry *dentry,
return;
}
+static inline int evm_inode_init_security(struct inode *inode,
+ const struct xattr *xattr_array,
+ struct xattr *evm)
+{
+ return -EOPNOTSUPP;
+}
+
#endif /* CONFIG_EVM_H */
#endif /* LINUX_EVM_H */
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 375dc3e..a45d0d6 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"
@@ -29,5 +30,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, const 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 c631b99..c9902bd 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -157,6 +157,26 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
return rc;
}
+int evm_init_hmac(struct inode *inode, const 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 1746c36..2348635 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -98,6 +98,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;
}
@@ -245,6 +251,38 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
return;
}
+/*
+ * evm_inode_init_security - initializes security.evm
+ */
+int evm_inode_init_security(struct inode *inode,
+ const struct xattr *lsm_xattr,
+ struct xattr *evm_xattr)
+{
+ struct evm_ima_xattr_data *xattr_data;
+ int rc;
+
+ if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name))
+ return -EOPNOTSUPP;
+
+ xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
+ if (!xattr_data)
+ return -ENOMEM;
+
+ xattr_data->type = EVM_XATTR_HMAC;
+ rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest);
+ if (rc < 0)
+ goto out;
+
+ evm_xattr->value = xattr_data;
+ evm_xattr->value_len = sizeof(*xattr_data);
+ evm_xattr->name = kstrdup(XATTR_EVM_SUFFIX, GFP_NOFS);
+ return 0;
+out:
+ kfree(xattr_data);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(evm_inode_init_security);
+
static struct crypto_hash *tfm_hmac; /* preload crypto alg */
static int __init init_evm(void)
{
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 10/16] evm: call evm_inode_init_security from security_inode_init_security
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (8 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 09/16] evm: add evm_inode_init_security to initialize new files Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 11/16] evm: crypto hash replaced by shash Mimi Zohar
` (8 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
Changelog v7:
- moved the initialization call to security_inode_init_security,
renaming evm_inode_post_init_security to evm_inode_init_security
- increase size of xattr array for EVM xattr
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
security/security.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/security/security.c b/security/security.c
index 21a79b3..181990a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -20,7 +20,7 @@
#include <linux/ima.h>
#include <linux/evm.h>
-#define MAX_LSM_XATTR 1
+#define MAX_LSM_EVM_XATTR 2
/* Boot-time LSM user choice */
static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
@@ -346,8 +346,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
const initxattrs initxattrs, void *fs_data)
{
- struct xattr new_xattrs[MAX_LSM_XATTR + 1];
- struct xattr *lsm_xattr;
+ struct xattr new_xattrs[MAX_LSM_EVM_XATTR + 1];
+ struct xattr *lsm_xattr, *evm_xattr, *xattr;
int ret;
if (unlikely(IS_PRIVATE(inode)))
@@ -364,11 +364,17 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
&lsm_xattr->value_len);
if (ret)
goto out;
+
+ evm_xattr = lsm_xattr + 1;
+ ret = evm_inode_init_security(inode, lsm_xattr, evm_xattr);
+ if (ret)
+ goto out;
ret = initxattrs(inode, new_xattrs, fs_data);
out:
- kfree(lsm_xattr->name);
- kfree(lsm_xattr->value);
-
+ for (xattr = new_xattrs; xattr->name != NULL; xattr++) {
+ kfree(xattr->name);
+ kfree(xattr->value);
+ }
return (ret == -EOPNOTSUPP) ? 0 : ret;
}
EXPORT_SYMBOL(security_inode_init_security);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 11/16] evm: crypto hash replaced by shash
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (9 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 10/16] evm: call evm_inode_init_security from security_inode_init_security Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 12/16] evm: additional parameter to pass integrity cache entry 'iint' Mimi Zohar
` (7 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Dmitry Kasatkin, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
From: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Using shash is more efficient, because the algorithm is allocated only
once. Only the descriptor to store the hash state needs to be allocated
for every operation.
Changelog v6:
- check for crypto_shash_setkey failure
Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
security/integrity/evm/evm.h | 2 +
security/integrity/evm/evm_crypto.c | 96 +++++++++++++++++++----------------
security/integrity/evm/evm_main.c | 6 +-
3 files changed, 58 insertions(+), 46 deletions(-)
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index a45d0d6..d320f51 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -19,6 +19,8 @@
extern int evm_initialized;
extern char *evm_hmac;
+extern struct crypto_shash *hmac_tfm;
+
/* List of EVM protected security xattrs */
extern char *evm_config_xattrnames[];
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index c9902bd..a57e0f0 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -16,8 +16,8 @@
#include <linux/module.h>
#include <linux/crypto.h>
#include <linux/xattr.h>
-#include <linux/scatterlist.h>
#include <keys/encrypted-type.h>
+#include <crypto/hash.h>
#include "evm.h"
#define EVMKEY "evm-key"
@@ -25,26 +25,42 @@
static unsigned char evmkey[MAX_KEY_SIZE];
static int evmkey_len = MAX_KEY_SIZE;
-static int init_desc(struct hash_desc *desc)
+struct crypto_shash *hmac_tfm;
+
+static struct shash_desc *init_desc(void)
{
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;
+ struct shash_desc *desc;
+
+ if (hmac_tfm == NULL) {
+ hmac_tfm = crypto_alloc_shash(evm_hmac, 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(hmac_tfm)) {
+ pr_err("Can not allocate %s (reason: %ld)\n",
+ evm_hmac, PTR_ERR(hmac_tfm));
+ rc = PTR_ERR(hmac_tfm);
+ hmac_tfm = NULL;
+ return ERR_PTR(rc);
+ }
}
- desc->flags = 0;
- rc = crypto_hash_setkey(desc->tfm, evmkey, evmkey_len);
+
+ desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(hmac_tfm),
+ GFP_KERNEL);
+ if (!desc)
+ return ERR_PTR(-ENOMEM);
+
+ desc->tfm = hmac_tfm;
+ desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+
+ rc = crypto_shash_setkey(hmac_tfm, evmkey, evmkey_len);
if (rc)
- goto out;
- rc = crypto_hash_init(desc);
+ goto out;
+ rc = crypto_shash_init(desc);
out:
- if (rc)
- crypto_free_hash(desc->tfm);
- return rc;
+ if (rc) {
+ kfree(desc);
+ return ERR_PTR(rc);
+ }
+ return desc;
}
/* Protect against 'cutting & pasting' security.evm xattr, include inode
@@ -53,7 +69,7 @@ out:
* (Additional directory/file metadata needs to be added for more complete
* protection.)
*/
-static void hmac_add_misc(struct hash_desc *desc, struct inode *inode,
+static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
char *digest)
{
struct h_misc {
@@ -63,7 +79,6 @@ static void hmac_add_misc(struct hash_desc *desc, struct inode *inode,
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;
@@ -71,9 +86,8 @@ static void hmac_add_misc(struct hash_desc *desc, struct inode *inode,
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);
+ crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc);
+ crypto_shash_final(desc, digest);
}
/*
@@ -88,8 +102,7 @@ int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
char *digest)
{
struct inode *inode = dentry->d_inode;
- struct hash_desc desc;
- struct scatterlist sg[1];
+ struct shash_desc *desc;
char **xattrname;
size_t xattr_size = 0;
char *xattr_value = NULL;
@@ -98,17 +111,17 @@ int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
if (!inode->i_op || !inode->i_op->getxattr)
return -EOPNOTSUPP;
- error = init_desc(&desc);
- if (error)
- return error;
+ desc = init_desc();
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
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);
+ crypto_shash_update(desc, (const u8 *)req_xattr_value,
+ req_xattr_value_len);
continue;
}
size = vfs_getxattr_alloc(dentry, *xattrname,
@@ -122,13 +135,13 @@ int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
error = 0;
xattr_size = size;
- sg_init_one(sg, xattr_value, xattr_size);
- crypto_hash_update(&desc, sg, xattr_size);
+ crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
}
- hmac_add_misc(&desc, inode, digest);
- kfree(xattr_value);
+ hmac_add_misc(desc, inode, digest);
+
out:
- crypto_free_hash(desc.tfm);
+ kfree(xattr_value);
+ kfree(desc);
return error;
}
@@ -160,20 +173,17 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
char *hmac_val)
{
- struct hash_desc desc;
- struct scatterlist sg[1];
- int error;
+ struct shash_desc *desc;
- error = init_desc(&desc);
- if (error != 0) {
+ desc = init_desc();
+ if (IS_ERR(desc)) {
printk(KERN_INFO "init_desc failed\n");
- return error;
+ return PTR_ERR(desc);
}
- 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);
+ crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
+ hmac_add_misc(desc, inode, hmac_val);
+ kfree(desc);
return 0;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 2348635..b65adb5 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -19,6 +19,7 @@
#include <linux/xattr.h>
#include <linux/integrity.h>
#include <linux/evm.h>
+#include <crypto/hash.h>
#include "evm.h"
int evm_initialized;
@@ -283,12 +284,10 @@ out:
}
EXPORT_SYMBOL_GPL(evm_inode_init_security);
-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");
@@ -301,7 +300,8 @@ err:
static void __exit cleanup_evm(void)
{
evm_cleanup_secfs();
- crypto_free_hash(tfm_hmac);
+ if (hmac_tfm)
+ crypto_free_shash(hmac_tfm);
}
/*
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 12/16] evm: additional parameter to pass integrity cache entry 'iint'
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (10 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 11/16] evm: crypto hash replaced by shash Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 13/16] evm: evm_verify_hmac must not return INTEGRITY_UNKNOWN Mimi Zohar
` (6 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Dmitry Kasatkin, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
From: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Additional iint parameter allows to skip lookup in the cache.
Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
include/linux/evm.h | 8 ++++++--
security/integrity/evm/evm_main.c | 18 ++++++++----------
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 7c10761..6d4e89b 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -11,11 +11,14 @@
#include <linux/integrity.h>
#include <linux/xattr.h>
+struct integrity_iint_cache;
+
#ifdef CONFIG_EVM
extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
const char *xattr_name,
void *xattr_value,
- size_t xattr_value_len);
+ size_t xattr_value_len,
+ struct integrity_iint_cache *iint);
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);
@@ -34,7 +37,8 @@ extern int evm_inode_init_security(struct inode *inode,
static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
const char *xattr_name,
void *xattr_value,
- size_t xattr_value_len)
+ size_t xattr_value_len,
+ struct integrity_iint_cache *iint)
{
return INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b65adb5..0fa8261 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -127,21 +127,19 @@ static int evm_protected_xattr(const char *req_xattr_name)
*/
enum integrity_status evm_verifyxattr(struct dentry *dentry,
const char *xattr_name,
- void *xattr_value, size_t xattr_value_len)
+ void *xattr_value, size_t xattr_value_len,
+ struct integrity_iint_cache *iint)
{
- 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(inode);
- if (!iint)
- return INTEGRITY_UNKNOWN;
- status = evm_verify_hmac(dentry, xattr_name, xattr_value,
+ if (!iint) {
+ iint = integrity_iint_find(dentry->d_inode);
+ if (!iint)
+ return INTEGRITY_UNKNOWN;
+ }
+ return evm_verify_hmac(dentry, xattr_name, xattr_value,
xattr_value_len, iint);
- return status;
}
EXPORT_SYMBOL_GPL(evm_verifyxattr);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 13/16] evm: evm_verify_hmac must not return INTEGRITY_UNKNOWN
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (11 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 12/16] evm: additional parameter to pass integrity cache entry 'iint' Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 14/16] evm: replace hmac_status with evm_status Mimi Zohar
` (5 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Dmitry Kasatkin, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
From: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
If EVM is not supported or enabled, evm_verify_hmac() returns
INTEGRITY_UNKNOWN, which ima_appraise_measurement() ignores and sets
the appraisal status based solely on the security.ima verification.
evm_verify_hmac() also returns INTEGRITY_UNKNOWN for other failures, such
as temporary failures like -ENOMEM, resulting in possible attack vectors.
This patch changes the default return code for temporary/unexpected
failures, like -ENOMEM, from INTEGRITY_UNKNOWN to INTEGRITY_FAIL, making
evm_verify_hmac() fail safe.
As a result, failures need to be re-evaluated in order to catch both
temporary errors, such as the -ENOMEM, as well as errors that have been
resolved in fix mode.
Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
security/integrity/evm/evm_main.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 0fa8261..bfe44df 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -56,13 +56,15 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
struct evm_ima_xattr_data xattr_data;
int rc;
- if (iint->hmac_status != INTEGRITY_UNKNOWN)
+ if (iint->hmac_status == INTEGRITY_PASS)
return iint->hmac_status;
+ /* if status is not PASS, try to check again - against -ENOMEM */
+
rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
xattr_value_len, xattr_data.digest);
if (rc < 0)
- return INTEGRITY_UNKNOWN;
+ goto err_out;
xattr_data.type = EVM_XATTR_HMAC;
rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, (u8 *)&xattr_data,
@@ -77,11 +79,8 @@ err_out:
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;
+ iint->hmac_status = INTEGRITY_FAIL;
}
return iint->hmac_status;
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 14/16] evm: replace hmac_status with evm_status
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (12 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 13/16] evm: evm_verify_hmac must not return INTEGRITY_UNKNOWN Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 15/16] evm: permit only valid security.evm xattrs to be updated Mimi Zohar
` (4 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Dmitry Kasatkin, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
From: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
We will use digital signatures in addtion to hmac.
Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
security/integrity/evm/evm_main.c | 14 +++++++-------
security/integrity/iint.c | 2 +-
security/integrity/integrity.h | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index bfe44df..eb07f9d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -56,8 +56,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
struct evm_ima_xattr_data xattr_data;
int rc;
- if (iint->hmac_status == INTEGRITY_PASS)
- return iint->hmac_status;
+ if (iint->evm_status == INTEGRITY_PASS)
+ return iint->evm_status;
/* if status is not PASS, try to check again - against -ENOMEM */
@@ -71,18 +71,18 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
sizeof xattr_data, GFP_NOFS);
if (rc < 0)
goto err_out;
- iint->hmac_status = INTEGRITY_PASS;
- return iint->hmac_status;
+ iint->evm_status = INTEGRITY_PASS;
+ return iint->evm_status;
err_out:
switch (rc) {
case -ENODATA: /* file not labelled */
- iint->hmac_status = INTEGRITY_NOLABEL;
+ iint->evm_status = INTEGRITY_NOLABEL;
break;
default:
- iint->hmac_status = INTEGRITY_FAIL;
+ iint->evm_status = INTEGRITY_FAIL;
}
- return iint->hmac_status;
+ return iint->evm_status;
}
static int evm_protected_xattr(const char *req_xattr_name)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 991df20..0a23e07 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -157,7 +157,7 @@ static void init_once(void *foo)
iint->version = 0;
iint->flags = 0UL;
mutex_init(&iint->mutex);
- iint->hmac_status = INTEGRITY_UNKNOWN;
+ iint->evm_status = INTEGRITY_UNKNOWN;
}
static int __init integrity_iintcache_init(void)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 7efbf56..880bbee 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -37,7 +37,7 @@ struct integrity_iint_cache {
unsigned char flags;
u8 digest[SHA1_DIGEST_SIZE];
struct mutex mutex; /* protects: version, flags, digest */
- enum integrity_status hmac_status;
+ enum integrity_status evm_status;
};
/* rbtree tree calls to lookup, insert, delete
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 15/16] evm: permit only valid security.evm xattrs to be updated
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (13 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 14/16] evm: replace hmac_status with evm_status Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 16/16] evm: add evm_inode_setattr to prevent updating an invalid security.evm Mimi Zohar
` (3 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
In addition to requiring CAP_SYS_ADMIN permission to modify/delete
security.evm, prohibit invalid security.evm xattrs from changing,
unless in fixmode. This patch prevents inadvertent 'fixing' of
security.evm to reflect offline modifications.
Changelog v7:
- rename boot paramater 'evm_mode' to 'evm'
Reported-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
Documentation/kernel-parameters.txt | 6 +++
security/integrity/evm/evm_main.c | 77 ++++++++++++++++++++++++++++------
2 files changed, 69 insertions(+), 14 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fd248a31..db97ff1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -48,6 +48,7 @@ parameter is applicable:
EDD BIOS Enhanced Disk Drive Services (EDD) is enabled
EFI EFI Partitioning (GPT) is enabled
EIDE EIDE/ATAPI support is enabled.
+ EVM Extended Verification Module
FB The frame buffer device is enabled.
GCOV GCOV profiling is enabled.
HW Appropriate hardware is enabled.
@@ -750,6 +751,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
This option is obsoleted by the "netdev=" option, which
has equivalent usage. See its documentation for details.
+ evm= [EVM]
+ Format: { "fix" }
+ Permit 'security.evm' to be updated regardless of
+ current integrity status.
+
failslab=
fail_page_alloc=
fail_make_request=[KNL]
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index eb07f9d..94d66af 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -37,13 +37,25 @@ char *evm_config_xattrnames[] = {
NULL
};
+static int evm_fixmode;
+static int __init evm_set_fixmode(char *str)
+{
+ if (strncmp(str, "fix", 3) == 0)
+ evm_fixmode = 1;
+ return 0;
+}
+__setup("evm=", evm_set_fixmode);
+
/*
* 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.)
+ * and compare it against the stored security.evm xattr.
+ *
+ * For performance:
+ * - use the previoulsy retrieved xattr value and length to calculate the
+ * HMAC.)
+ * - cache the verification result in the iint, when available.
*
* Returns integrity status
*/
@@ -54,9 +66,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
struct integrity_iint_cache *iint)
{
struct evm_ima_xattr_data xattr_data;
+ enum integrity_status evm_status;
int rc;
- if (iint->evm_status == INTEGRITY_PASS)
+ if (iint && iint->evm_status == INTEGRITY_PASS)
return iint->evm_status;
/* if status is not PASS, try to check again - against -ENOMEM */
@@ -71,18 +84,21 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
sizeof xattr_data, GFP_NOFS);
if (rc < 0)
goto err_out;
- iint->evm_status = INTEGRITY_PASS;
- return iint->evm_status;
+ evm_status = INTEGRITY_PASS;
+ goto out;
err_out:
switch (rc) {
case -ENODATA: /* file not labelled */
- iint->evm_status = INTEGRITY_NOLABEL;
+ evm_status = INTEGRITY_NOLABEL;
break;
default:
- iint->evm_status = INTEGRITY_FAIL;
+ evm_status = INTEGRITY_FAIL;
}
- return iint->evm_status;
+out:
+ if (iint)
+ iint->evm_status = evm_status;
+ return evm_status;
}
static int evm_protected_xattr(const char *req_xattr_name)
@@ -157,6 +173,22 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name,
return 0;
}
+/*
+ * evm_verify_current_integrity - verify the dentry's metadata integrity
+ * @dentry: pointer to the affected dentry
+ *
+ * Verify and return the dentry's metadata integrity. The exceptions are
+ * before EVM is initialized or in 'fix' mode.
+ */
+static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
+{
+ struct inode *inode = dentry->d_inode;
+
+ if (!evm_initialized || !S_ISREG(inode->i_mode) || evm_fixmode)
+ return 0;
+ return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);
+}
+
/**
* evm_inode_setxattr - protect the EVM extended attribute
* @dentry: pointer to the affected dentry
@@ -164,13 +196,22 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_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
+ * Updating 'security.evm' requires CAP_SYS_ADMIN privileges and that
+ * the current value is valid.
*/
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);
+
+ enum integrity_status evm_status;
+ int ret;
+
+ ret = evm_protect_xattr(dentry, xattr_name, xattr_value,
+ xattr_value_len);
+ if (ret)
+ return ret;
+ evm_status = evm_verify_current_integrity(dentry);
+ return evm_status == INTEGRITY_PASS ? 0 : -EPERM;
}
/**
@@ -178,11 +219,19 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
* @dentry: pointer to the affected dentry
* @xattr_name: pointer to the affected extended attribute name
*
- * Prevent 'security.evm' from being removed.
+ * Removing 'security.evm' requires CAP_SYS_ADMIN privileges and that
+ * the current value is valid.
*/
int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name)
{
- return evm_protect_xattr(dentry, xattr_name, NULL, 0);
+ enum integrity_status evm_status;
+ int ret;
+
+ ret = evm_protect_xattr(dentry, xattr_name, NULL, 0);
+ if (ret)
+ return ret;
+ evm_status = evm_verify_current_integrity(dentry);
+ return evm_status == INTEGRITY_PASS ? 0 : -EPERM;
}
/**
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v7 16/16] evm: add evm_inode_setattr to prevent updating an invalid security.evm
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (14 preceding siblings ...)
2011-06-29 19:50 ` [PATCH v7 15/16] evm: permit only valid security.evm xattrs to be updated Mimi Zohar
@ 2011-06-29 19:50 ` Mimi Zohar
2011-06-29 20:53 ` Kyle Moffett
` (2 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 19:50 UTC (permalink / raw)
To: linux-security-module
Cc: Mimi Zohar, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin,
Mimi Zohar
Permit changing of security.evm only when valid, unless in fixmode.
Reported-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
include/linux/evm.h | 6 ++++++
security/integrity/evm/evm_main.c | 15 +++++++++++++++
security/security.c | 7 ++++++-
3 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 6d4e89b..db5556d 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -19,6 +19,7 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
void *xattr_value,
size_t xattr_value_len,
struct integrity_iint_cache *iint);
+extern int evm_inode_setattr(struct dentry *dentry, struct iattr *attr);
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);
@@ -44,6 +45,11 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
}
#endif
+static int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
+{
+ return 0;
+}
+
static inline void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
{
return;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 94d66af..8fc5b5d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -278,6 +278,21 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
}
/**
+ * evm_inode_setattr - prevent updating an invalid EVM extended attribute
+ * @dentry: pointer to the affected dentry
+ */
+int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
+{
+ unsigned int ia_valid = attr->ia_valid;
+ enum integrity_status evm_status;
+
+ if (ia_valid & ~(ATTR_MODE | ATTR_UID | ATTR_GID))
+ return 0;
+ evm_status = evm_verify_current_integrity(dentry);
+ return evm_status == INTEGRITY_PASS ? 0 : -EPERM;
+}
+
+/**
* evm_inode_post_setattr - update 'security.evm' after modifying metadata
* @dentry: pointer to the affected dentry
* @ia_valid: for the UID and GID status
diff --git a/security/security.c b/security/security.c
index 181990a..19251cc 100644
--- a/security/security.c
+++ b/security/security.c
@@ -571,9 +571,14 @@ int security_inode_exec_permission(struct inode *inode, unsigned int flags)
int security_inode_setattr(struct dentry *dentry, struct iattr *attr)
{
+ int ret;
+
if (unlikely(IS_PRIVATE(dentry->d_inode)))
return 0;
- return security_ops->inode_setattr(dentry, attr);
+ ret = security_ops->inode_setattr(dentry, attr);
+ if (ret)
+ return ret;
+ return evm_inode_setattr(dentry, attr);
}
EXPORT_SYMBOL_GPL(security_inode_setattr);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
@ 2011-06-29 20:53 ` Kyle Moffett
2011-06-29 19:50 ` [PATCH v7 02/16] integrity: move ima inode integrity data management Mimi Zohar
` (17 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Kyle Moffett @ 2011-06-29 20:53 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> protect the integrity of a running system from unauthorized changes. When
> these protections are not running, such as when booting a malicious OS,
> mounting the disk under a different operating system, or physically moving
> the disk to another system, an "offline" attack is free to read and write
> file data/metadata.
>
> 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 the IMA-appraisal
> patchset, integrity appraisal decisions. This patchset provides the framework
> and an initial method to detect offline tampering of the security extended
> attributes. The initial method 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'. Other methods of validating the integrity
> of a file's metadata will be posted separately (eg. EVM-digital-signatures).
Hmm, I'm not sure that this design actually provides the protection that
you claim it does.
Specifically, you don't actually protect the on-disk data-structures that
are far more vulnerable to malicious modification than the actual *values*
of the extended attributes themselves.
For example, compare the number of actual practical exploits of image
buffer-overflow vulnerabilities involving user-generated content versus
the number of Man-in-the-middle exploits of trusted content, I guarantee
you there are a lot more of the former in the wild.
It's also worth mentioning that one of the reasons we have traditionally
prevented user mounts of many filesystems is because the filesystem
drivers tended to be buggy and susceptible to panic or buffer overflow.
To give you a trivial example of how this could be subverted, you could
modify the block mapping table of an unprotected file to reference some
blocks used in a "protected" file. Then boot up the system and "verify"
the protected file, and then write new data to the unprotected file and
generate some memory pressure to force the protected data to be reloaded
from disk.
Another good example of how this breaks down in practice would be the
various incompatibilities in different VFAT filesystem behavior with
NUL filenames. I don't remember what the change was on Linux, but in
practice on several operating systems you can "mkdir" a new directory
entry and several more could suddenly appear (because the NUL entry is
now populated).
If you want to use HMAC verification of disk contents then you should not
be performing the HMAC of some partial logical view of them, but instead
protect the *disk contents* themselves!!!
Perhaps modifying dm-crypt to support HMAC-based authentication modes
would be one approach. You could probably even code it to store the HMAC
metadata in a separate partition to allow for read-only access by your
bootloader, etc.
Cheers,
Kyle Moffett
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
@ 2011-06-29 20:53 ` Kyle Moffett
0 siblings, 0 replies; 41+ messages in thread
From: Kyle Moffett @ 2011-06-29 20:53 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> protect the integrity of a running system from unauthorized changes. When
> these protections are not running, such as when booting a malicious OS,
> mounting the disk under a different operating system, or physically moving
> the disk to another system, an "offline" attack is free to read and write
> file data/metadata.
>
> 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 the IMA-appraisal
> patchset, integrity appraisal decisions. This patchset provides the framework
> and an initial method to detect offline tampering of the security extended
> attributes. The initial method 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'. Other methods of validating the integrity
> of a file's metadata will be posted separately (eg. EVM-digital-signatures).
Hmm, I'm not sure that this design actually provides the protection that
you claim it does.
Specifically, you don't actually protect the on-disk data-structures that
are far more vulnerable to malicious modification than the actual *values*
of the extended attributes themselves.
For example, compare the number of actual practical exploits of image
buffer-overflow vulnerabilities involving user-generated content versus
the number of Man-in-the-middle exploits of trusted content, I guarantee
you there are a lot more of the former in the wild.
It's also worth mentioning that one of the reasons we have traditionally
prevented user mounts of many filesystems is because the filesystem
drivers tended to be buggy and susceptible to panic or buffer overflow.
To give you a trivial example of how this could be subverted, you could
modify the block mapping table of an unprotected file to reference some
blocks used in a "protected" file. Then boot up the system and "verify"
the protected file, and then write new data to the unprotected file and
generate some memory pressure to force the protected data to be reloaded
from disk.
Another good example of how this breaks down in practice would be the
various incompatibilities in different VFAT filesystem behavior with
NUL filenames. I don't remember what the change was on Linux, but in
practice on several operating systems you can "mkdir" a new directory
entry and several more could suddenly appear (because the NUL entry is
now populated).
If you want to use HMAC verification of disk contents then you should not
be performing the HMAC of some partial logical view of them, but instead
protect the *disk contents* themselves!!!
Perhaps modifying dm-crypt to support HMAC-based authentication modes
would be one approach. You could probably even code it to store the HMAC
metadata in a separate partition to allow for read-only access by your
bootloader, etc.
Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-29 20:53 ` Kyle Moffett
(?)
@ 2011-06-29 23:42 ` Mimi Zohar
2011-06-30 1:57 ` Kyle Moffett
-1 siblings, 1 reply; 41+ messages in thread
From: Mimi Zohar @ 2011-06-29 23:42 UTC (permalink / raw)
To: Kyle Moffett
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> > protect the integrity of a running system from unauthorized changes. When
> > these protections are not running, such as when booting a malicious OS,
> > mounting the disk under a different operating system, or physically moving
> > the disk to another system, an "offline" attack is free to read and write
> > file data/metadata.
> >
> > 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 the IMA-appraisal
> > patchset, integrity appraisal decisions. This patchset provides the framework
> > and an initial method to detect offline tampering of the security extended
> > attributes. The initial method 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'. Other methods of validating the integrity
> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
>
> Hmm, I'm not sure that this design actually provides the protection that
> you claim it does.
>
> Specifically, you don't actually protect the on-disk data-structures that
> are far more vulnerable to malicious modification than the actual *values*
> of the extended attributes themselves.
True, EVM only protects the file metadata. The patch description says,
While this patchset does authenticate the security xattrs, and
cryptographically binds them to the inode, coming extensions
will bind other directory and inode metadata for more complete
protection.
It should have said, "bind other directory, inode data and inode
metadata."
In particular, IMA-appraisal stores the file data's hash as the
security.ima xattr, which is EVM protected. Other methods, such as
digital signatures, could be used instead of the file's hash, to
additionally provide authenticity.
thanks,
Mimi
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
2011-06-29 23:42 ` Mimi Zohar
@ 2011-06-30 1:57 ` Kyle Moffett
0 siblings, 0 replies; 41+ messages in thread
From: Kyle Moffett @ 2011-06-30 1:57 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
>> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
>> > protect the integrity of a running system from unauthorized changes. When
>> > these protections are not running, such as when booting a malicious OS,
>> > mounting the disk under a different operating system, or physically moving
>> > the disk to another system, an "offline" attack is free to read and write
>> > file data/metadata.
>> >
>> > 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 the IMA-appraisal
>> > patchset, integrity appraisal decisions. This patchset provides the framework
>> > and an initial method to detect offline tampering of the security extended
>> > attributes. The initial method 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'. Other methods of validating the integrity
>> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
>>
>> Hmm, I'm not sure that this design actually provides the protection that
>> you claim it does.
>>
>> Specifically, you don't actually protect the on-disk data-structures that
>> are far more vulnerable to malicious modification than the actual *values*
>> of the extended attributes themselves.
>
> True, EVM only protects the file metadata. The patch description says,
>
> While this patchset does authenticate the security xattrs, and
> cryptographically binds them to the inode, coming extensions
> will bind other directory and inode metadata for more complete
> protection.
>
> It should have said, "bind other directory, inode data and inode
> metadata."
>
> In particular, IMA-appraisal stores the file data's hash as the
> security.ima xattr, which is EVM protected. Other methods, such as
> digital signatures, could be used instead of the file's hash, to
> additionally provide authenticity.
The problem is that your *design* assumes that the filesystem itself is
valid, but your stated threat model assumes that the attacker has offline
access to the filesystem, an explicit contradiction.
There have been numerous cases in the past where a corrupt or invalid
filesystem causes kernel panics or even exploitable overflows or memory
corruption; see the history of the "fsfuzzer" tool for more information.
Furthermore, if the attacker can intentionally cause data extent or inode
extended attribute aliasing (shared space-on-disk) between different
files then your entire security model falls flat.
So if you assume the attacker has raw access to the underlying filesystem
then you MUST authenticate *all* of the low-level filesystem data,
including the "implicit" metadata of allocation tables, extents, etc.
Cheers,
Kyle Moffett
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
@ 2011-06-30 1:57 ` Kyle Moffett
0 siblings, 0 replies; 41+ messages in thread
From: Kyle Moffett @ 2011-06-30 1:57 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
>> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
>> > protect the integrity of a running system from unauthorized changes. When
>> > these protections are not running, such as when booting a malicious OS,
>> > mounting the disk under a different operating system, or physically moving
>> > the disk to another system, an "offline" attack is free to read and write
>> > file data/metadata.
>> >
>> > 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 the IMA-appraisal
>> > patchset, integrity appraisal decisions. This patchset provides the framework
>> > and an initial method to detect offline tampering of the security extended
>> > attributes. The initial method 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'. Other methods of validating the integrity
>> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
>>
>> Hmm, I'm not sure that this design actually provides the protection that
>> you claim it does.
>>
>> Specifically, you don't actually protect the on-disk data-structures that
>> are far more vulnerable to malicious modification than the actual *values*
>> of the extended attributes themselves.
>
> True, EVM only protects the file metadata. The patch description says,
>
> While this patchset does authenticate the security xattrs, and
> cryptographically binds them to the inode, coming extensions
> will bind other directory and inode metadata for more complete
> protection.
>
> It should have said, "bind other directory, inode data and inode
> metadata."
>
> In particular, IMA-appraisal stores the file data's hash as the
> security.ima xattr, which is EVM protected. Other methods, such as
> digital signatures, could be used instead of the file's hash, to
> additionally provide authenticity.
The problem is that your *design* assumes that the filesystem itself is
valid, but your stated threat model assumes that the attacker has offline
access to the filesystem, an explicit contradiction.
There have been numerous cases in the past where a corrupt or invalid
filesystem causes kernel panics or even exploitable overflows or memory
corruption; see the history of the "fsfuzzer" tool for more information.
Furthermore, if the attacker can intentionally cause data extent or inode
extended attribute aliasing (shared space-on-disk) between different
files then your entire security model falls flat.
So if you assume the attacker has raw access to the underlying filesystem
then you MUST authenticate *all* of the low-level filesystem data,
including the "implicit" metadata of allocation tables, extents, etc.
Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-30 1:57 ` Kyle Moffett
(?)
@ 2011-06-30 3:51 ` Mimi Zohar
2011-06-30 22:32 ` Kyle Moffett
[not found] ` <BANLkTin-x1kkXiowUYjBS_tr4iwDrzNQkA@mail.gmail.com>
-1 siblings, 2 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-30 3:51 UTC (permalink / raw)
To: Kyle Moffett
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford
On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> >> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> >> > protect the integrity of a running system from unauthorized changes. When
> >> > these protections are not running, such as when booting a malicious OS,
> >> > mounting the disk under a different operating system, or physically moving
> >> > the disk to another system, an "offline" attack is free to read and write
> >> > file data/metadata.
> >> >
> >> > 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 the IMA-appraisal
> >> > patchset, integrity appraisal decisions. This patchset provides the framework
> >> > and an initial method to detect offline tampering of the security extended
> >> > attributes. The initial method 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'. Other methods of validating the integrity
> >> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
> >>
> >> Hmm, I'm not sure that this design actually provides the protection that
> >> you claim it does.
> >>
> >> Specifically, you don't actually protect the on-disk data-structures that
> >> are far more vulnerable to malicious modification than the actual *values*
> >> of the extended attributes themselves.
> >
> > True, EVM only protects the file metadata. The patch description says,
> >
> > While this patchset does authenticate the security xattrs, and
> > cryptographically binds them to the inode, coming extensions
> > will bind other directory and inode metadata for more complete
> > protection.
> >
> > It should have said, "bind other directory, inode data and inode
> > metadata."
> >
> > In particular, IMA-appraisal stores the file data's hash as the
> > security.ima xattr, which is EVM protected. Other methods, such as
> > digital signatures, could be used instead of the file's hash, to
> > additionally provide authenticity.
>
> The problem is that your *design* assumes that the filesystem itself is
> valid, but your stated threat model assumes that the attacker has offline
> access to the filesystem, an explicit contradiction.
>
> There have been numerous cases in the past where a corrupt or invalid
> filesystem causes kernel panics or even exploitable overflows or memory
> corruption; see the history of the "fsfuzzer" tool for more information.
>
> Furthermore, if the attacker can intentionally cause data extent or inode
> extended attribute aliasing (shared space-on-disk) between different
> files then your entire security model falls flat.
>
> So if you assume the attacker has raw access to the underlying filesystem
> then you MUST authenticate *all* of the low-level filesystem data,
> including the "implicit" metadata of allocation tables, extents, etc.
>
> Cheers,
> Kyle Moffett
Assuming someone does modify the underlying filesystem, how does that
break the security model? The purpose of EVM/IMA-appraisal is not to
prevent files offline from being modified, but to detect if/when it
occurs and to enforce file integrity.
Mimi
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
2011-06-30 3:51 ` Mimi Zohar
@ 2011-06-30 22:32 ` Kyle Moffett
[not found] ` <BANLkTin-x1kkXiowUYjBS_tr4iwDrzNQkA@mail.gmail.com>
1 sibling, 0 replies; 41+ messages in thread
From: Kyle Moffett @ 2011-06-30 22:32 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford
Whoops, resent in plain text, sorry about the HTML
On Wed, Jun 29, 2011 at 23:51, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
>> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
>> >> Hmm, I'm not sure that this design actually provides the protection that
>> >> you claim it does.
>> >>
>> >> Specifically, you don't actually protect the on-disk data-structures that
>> >> are far more vulnerable to malicious modification than the actual *values*
>> >> of the extended attributes themselves.
>> >
>> > True, EVM only protects the file metadata. The patch description says,
>> >
>> > While this patchset does authenticate the security xattrs, and
>> > cryptographically binds them to the inode, coming extensions
>> > will bind other directory and inode metadata for more complete
>> > protection.
>> >
>> > It should have said, "bind other directory, inode data and inode
>> > metadata."
>> >
>> > In particular, IMA-appraisal stores the file data's hash as the
>> > security.ima xattr, which is EVM protected. Other methods, such as
>> > digital signatures, could be used instead of the file's hash, to
>> > additionally provide authenticity.
>>
>> The problem is that your *design* assumes that the filesystem itself is
>> valid, but your stated threat model assumes that the attacker has offline
>> access to the filesystem, an explicit contradiction.
>>
>> There have been numerous cases in the past where a corrupt or invalid
>> filesystem causes kernel panics or even exploitable overflows or memory
>> corruption; see the history of the "fsfuzzer" tool for more information.
>>
>> Furthermore, if the attacker can intentionally cause data extent or inode
>> extended attribute aliasing (shared space-on-disk) between different
>> files then your entire security model falls flat.
>>
>> So if you assume the attacker has raw access to the underlying filesystem
>> then you MUST authenticate *all* of the low-level filesystem data,
>> including the "implicit" metadata of allocation tables, extents, etc.
>>
>> Cheers,
>> Kyle Moffett
>
> Assuming someone does modify the underlying filesystem, how does that
> break the security model? The purpose of EVM/IMA-appraisal is not to
> prevent files offline from being modified, but to detect if/when it
> occurs and to enforce file integrity.
The problem is that you are assuming that a large chunk of filesystem
code is capable of properly and securely handling untrusted and malicious
content. Historically filesystem drivers are NOT capable of handling
such things, as evidenced by the large number of bugs that tools such as
fsfuzzer tend to trigger. If you want to use IMA as-designed then you
need to perform a relatively extensive audit of filesystem and fsck code.
Furthermore, even when the filesystem does not have any security issues
itself, you are assuming that intentionally malicious data-aliasing
between "trusted" and "untrusted" files can have no potential security
implications. You should look at the prevalence of simple stupid "/tmp"
symlink attacks for more counter-examples there.
In addition, IMA relies on the underlying attribute and data caching
properties of the VFS, which won't hold true for intentionally malicious
corrupted filesystems. It effectively assumes that writing data or
metadata for one file will not invalidate the cached data or metadata for
another which is blatantly false when filesystem extents overlap each
other.
Overall, the IMA architecture assumes that if it loads and validates the
file data or metadata that it cannot be changed except through a kernel
access to that particular inode. For a corrupted filesystem that is
absolutely untrue.
Cheers,
Kyle Moffett
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
@ 2011-06-30 22:32 ` Kyle Moffett
0 siblings, 0 replies; 41+ messages in thread
From: Kyle Moffett @ 2011-06-30 22:32 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford
Whoops, resent in plain text, sorry about the HTML
On Wed, Jun 29, 2011 at 23:51, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
>> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
>> >> Hmm, I'm not sure that this design actually provides the protection that
>> >> you claim it does.
>> >>
>> >> Specifically, you don't actually protect the on-disk data-structures that
>> >> are far more vulnerable to malicious modification than the actual *values*
>> >> of the extended attributes themselves.
>> >
>> > True, EVM only protects the file metadata. The patch description says,
>> >
>> > While this patchset does authenticate the security xattrs, and
>> > cryptographically binds them to the inode, coming extensions
>> > will bind other directory and inode metadata for more complete
>> > protection.
>> >
>> > It should have said, "bind other directory, inode data and inode
>> > metadata."
>> >
>> > In particular, IMA-appraisal stores the file data's hash as the
>> > security.ima xattr, which is EVM protected. Other methods, such as
>> > digital signatures, could be used instead of the file's hash, to
>> > additionally provide authenticity.
>>
>> The problem is that your *design* assumes that the filesystem itself is
>> valid, but your stated threat model assumes that the attacker has offline
>> access to the filesystem, an explicit contradiction.
>>
>> There have been numerous cases in the past where a corrupt or invalid
>> filesystem causes kernel panics or even exploitable overflows or memory
>> corruption; see the history of the "fsfuzzer" tool for more information.
>>
>> Furthermore, if the attacker can intentionally cause data extent or inode
>> extended attribute aliasing (shared space-on-disk) between different
>> files then your entire security model falls flat.
>>
>> So if you assume the attacker has raw access to the underlying filesystem
>> then you MUST authenticate *all* of the low-level filesystem data,
>> including the "implicit" metadata of allocation tables, extents, etc.
>>
>> Cheers,
>> Kyle Moffett
>
> Assuming someone does modify the underlying filesystem, how does that
> break the security model? The purpose of EVM/IMA-appraisal is not to
> prevent files offline from being modified, but to detect if/when it
> occurs and to enforce file integrity.
The problem is that you are assuming that a large chunk of filesystem
code is capable of properly and securely handling untrusted and malicious
content. Historically filesystem drivers are NOT capable of handling
such things, as evidenced by the large number of bugs that tools such as
fsfuzzer tend to trigger. If you want to use IMA as-designed then you
need to perform a relatively extensive audit of filesystem and fsck code.
Furthermore, even when the filesystem does not have any security issues
itself, you are assuming that intentionally malicious data-aliasing
between "trusted" and "untrusted" files can have no potential security
implications. You should look at the prevalence of simple stupid "/tmp"
symlink attacks for more counter-examples there.
In addition, IMA relies on the underlying attribute and data caching
properties of the VFS, which won't hold true for intentionally malicious
corrupted filesystems. It effectively assumes that writing data or
metadata for one file will not invalidate the cached data or metadata for
another which is blatantly false when filesystem extents overlap each
other.
Overall, the IMA architecture assumes that if it loads and validates the
file data or metadata that it cannot be changed except through a kernel
access to that particular inode. For a corrupted filesystem that is
absolutely untrue.
Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-30 22:32 ` Kyle Moffett
(?)
@ 2011-07-14 15:07 ` David Safford
-1 siblings, 0 replies; 41+ messages in thread
From: David Safford @ 2011-07-14 15:07 UTC (permalink / raw)
To: Kyle Moffett
Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-fsdevel,
James Morris
On Thu, 2011-06-30 at 18:32 -0400, Kyle Moffett wrote:
> Whoops, resent in plain text, sorry about the HTML
>
> On Wed, Jun 29, 2011 at 23:51, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> >> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> >> >> Hmm, I'm not sure that this design actually provides the protection that
> >> >> you claim it does.
> >> >>
> >> >> Specifically, you don't actually protect the on-disk data-structures that
> >> >> are far more vulnerable to malicious modification than the actual *values*
> >> >> of the extended attributes themselves.
> >> >
> >> > True, EVM only protects the file metadata. The patch description says,
> >> >
> >> > While this patchset does authenticate the security xattrs, and
> >> > cryptographically binds them to the inode, coming extensions
> >> > will bind other directory and inode metadata for more complete
> >> > protection.
> >> >
> >> > It should have said, "bind other directory, inode data and inode
> >> > metadata."
> >> >
> >> > In particular, IMA-appraisal stores the file data's hash as the
> >> > security.ima xattr, which is EVM protected. Other methods, such as
> >> > digital signatures, could be used instead of the file's hash, to
> >> > additionally provide authenticity.
> >>
> >> The problem is that your *design* assumes that the filesystem itself is
> >> valid, but your stated threat model assumes that the attacker has offline
> >> access to the filesystem, an explicit contradiction.
> >>
> >> There have been numerous cases in the past where a corrupt or invalid
> >> filesystem causes kernel panics or even exploitable overflows or memory
> >> corruption; see the history of the "fsfuzzer" tool for more information.
> >>
> >> Furthermore, if the attacker can intentionally cause data extent or inode
> >> extended attribute aliasing (shared space-on-disk) between different
> >> files then your entire security model falls flat.
> >>
> >> So if you assume the attacker has raw access to the underlying filesystem
> >> then you MUST authenticate *all* of the low-level filesystem data,
> >> including the "implicit" metadata of allocation tables, extents, etc.
> >>
> >> Cheers,
> >> Kyle Moffett
> >
> > Assuming someone does modify the underlying filesystem, how does that
> > break the security model? The purpose of EVM/IMA-appraisal is not to
> > prevent files offline from being modified, but to detect if/when it
> > occurs and to enforce file integrity.
>
> The problem is that you are assuming that a large chunk of filesystem
> code is capable of properly and securely handling untrusted and malicious
> content. Historically filesystem drivers are NOT capable of handling
> such things, as evidenced by the large number of bugs that tools such as
> fsfuzzer tend to trigger. If you want to use IMA as-designed then you
> need to perform a relatively extensive audit of filesystem and fsck code.
Seems to me exploitable bugs in fs code should be fixed regardless of EVM...
> Furthermore, even when the filesystem does not have any security issues
> itself, you are assuming that intentionally malicious data-aliasing
> between "trusted" and "untrusted" files can have no potential security
> implications. You should look at the prevalence of simple stupid "/tmp"
> symlink attacks for more counter-examples there.
>
> In addition, IMA relies on the underlying attribute and data caching
> properties of the VFS, which won't hold true for intentionally malicious
> corrupted filesystems. It effectively assumes that writing data or
> metadata for one file will not invalidate the cached data or metadata for
> another which is blatantly false when filesystem extents overlap each
> other.
As discussed, this is a tradeoff in IMA-Appraisal-HASH, which allows
data in protected files to be updated. The attacks do not work on EVM,
IMA-Appraisal-DigitalSignature, or IMA attestation, where the attacker
cannot forge valid verifiers, even with arbitrary access to the raw device.
> Overall, the IMA architecture assumes that if it loads and validates the
> file data or metadata that it cannot be changed except through a kernel
> access to that particular inode. For a corrupted filesystem that is
> absolutely untrue.
IMA-Appraisal-HASH does as a deliberate tradeoff. The other integrity
modules do not.
thanks
dave
^ permalink raw reply [flat|nested] 41+ messages in thread
[parent not found: <BANLkTin-x1kkXiowUYjBS_tr4iwDrzNQkA@mail.gmail.com>]
* Re: [PATCH v7 00/16] EVM
[not found] ` <BANLkTin-x1kkXiowUYjBS_tr4iwDrzNQkA@mail.gmail.com>
@ 2011-07-01 14:34 ` Mimi Zohar
2011-07-01 21:55 ` Mimi Zohar
0 siblings, 1 reply; 41+ messages in thread
From: Mimi Zohar @ 2011-07-01 14:34 UTC (permalink / raw)
To: Kyle Moffett
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford
On Thu, 2011-06-30 at 18:31 -0400, Kyle Moffett wrote:
> The problem is that you are assuming that a large chunk of filesystem
> code is capable of properly and securely handling untrusted and
> malicious
> content. Historically filesystem drivers are NOT capable of handling
> such things, as evidenced by the large number of bugs that tools such
> as
> fsfuzzer tend to trigger. If you want to use IMA as-designed then you
> need to perform a relatively extensive audit of filesystem and fsck
> code.
>
> Furthermore, even when the filesystem does not have any security
> issues
> itself, you are assuming that intentionally malicious data-aliasing
> between "trusted" and "untrusted" files can have no potential security
> implications. You should look at the prevalence of simple stupid
> "/tmp"
> symlink attacks for more counter-examples there.
>
> In addition, IMA relies on the underlying attribute and data caching
> properties of the VFS, which won't hold true for intentionally
> malicious
> corrupted filesystems. It effectively assumes that writing data or
> metadata for one file will not invalidate the cached data or metadata
> for
> another which is blatantly false when filesystem extents overlap each
> other.
>
> Overall, the IMA architecture assumes that if it loads and validates
> the
> file data or metadata that it cannot be changed except through a
> kernel
> access to that particular inode. For a corrupted filesystem that is
> absolutely untrue.
>
> Cheers,
> Kyle Moffett
You've brought up a number of interesting scenarios, which I appreciate.
I will definitely take a closer look at fsfuzzer. It might be a good
starting point for an EVM/IMA-appraisal LTP testsuite. The bottom line,
as I said previously, is that EVM/IMA-appraisal doesn't need to prevent
these things from occurring. It just needs to be able to detect them.
Caching the integrity verification results is a performance issue, be it
an important one.
Currently the integrity verification results are reset when the file
data or metadata changes and removed on __fput(). Based on your
scenarios, I am looking to see if there might be additional situations
where the verification results need to be reset.
thanks,
Mimi
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
2011-07-01 14:34 ` Mimi Zohar
@ 2011-07-01 21:55 ` Mimi Zohar
0 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-07-01 21:55 UTC (permalink / raw)
To: Kyle Moffett
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford
On Fri, 2011-07-01 at 10:34 -0400, Mimi Zohar wrote:
> On Thu, 2011-06-30 at 18:31 -0400, Kyle Moffett wrote:
>
> > The problem is that you are assuming that a large chunk of filesystem
> > code is capable of properly and securely handling untrusted and
> > malicious
> > content. Historically filesystem drivers are NOT capable of handling
> > such things, as evidenced by the large number of bugs that tools such
> > as
> > fsfuzzer tend to trigger. If you want to use IMA as-designed then you
> > need to perform a relatively extensive audit of filesystem and fsck
> > code.
> >
> > Furthermore, even when the filesystem does not have any security
> > issues
> > itself, you are assuming that intentionally malicious data-aliasing
> > between "trusted" and "untrusted" files can have no potential security
> > implications. You should look at the prevalence of simple stupid
> > "/tmp"
> > symlink attacks for more counter-examples there.
> >
> > In addition, IMA relies on the underlying attribute and data caching
> > properties of the VFS, which won't hold true for intentionally
> > malicious
> > corrupted filesystems. It effectively assumes that writing data or
> > metadata for one file will not invalidate the cached data or metadata
> > for
> > another which is blatantly false when filesystem extents overlap each
> > other.
> >
> > Overall, the IMA architecture assumes that if it loads and validates
> > the
> > file data or metadata that it cannot be changed except through a
> > kernel
> > access to that particular inode. For a corrupted filesystem that is
> > absolutely untrue.
> >
> > Cheers,
> > Kyle Moffett
>
> You've brought up a number of interesting scenarios, which I appreciate.
> I will definitely take a closer look at fsfuzzer. It might be a good
> starting point for an EVM/IMA-appraisal LTP testsuite. The bottom line,
> as I said previously, is that EVM/IMA-appraisal doesn't need to prevent
> these things from occurring. It just needs to be able to detect them.
> Caching the integrity verification results is a performance issue, be it
> an important one.
>
> Currently the integrity verification results are reset when the file
> data or metadata changes and removed on __fput(). Based on your
> scenarios, I am looking to see if there might be additional situations
> where the verification results need to be reset.
I forgot to mention that the IMA-appraisal-directory extension,
discussed in the Integrity whitepaper, will also address some of the
concerns you raised.
thanks,
Mimi
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-30 1:57 ` Kyle Moffett
@ 2011-07-14 15:07 ` David Safford
-1 siblings, 0 replies; 41+ messages in thread
From: David Safford @ 2011-07-14 15:07 UTC (permalink / raw)
To: Kyle Moffett
Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-fsdevel,
James Morris, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> >> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> >> > protect the integrity of a running system from unauthorized changes. When
> >> > these protections are not running, such as when booting a malicious OS,
> >> > mounting the disk under a different operating system, or physically moving
> >> > the disk to another system, an "offline" attack is free to read and write
> >> > file data/metadata.
> >> >
> >> > 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 the IMA-appraisal
> >> > patchset, integrity appraisal decisions. This patchset provides the framework
> >> > and an initial method to detect offline tampering of the security extended
> >> > attributes. The initial method 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'. Other methods of validating the integrity
> >> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
> >>
> >> Hmm, I'm not sure that this design actually provides the protection that
> >> you claim it does.
> >>
> >> Specifically, you don't actually protect the on-disk data-structures that
> >> are far more vulnerable to malicious modification than the actual *values*
> >> of the extended attributes themselves.
> >
> > True, EVM only protects the file metadata. The patch description says,
> >
> > While this patchset does authenticate the security xattrs, and
> > cryptographically binds them to the inode, coming extensions
> > will bind other directory and inode metadata for more complete
> > protection.
> >
> > It should have said, "bind other directory, inode data and inode
> > metadata."
> >
> > In particular, IMA-appraisal stores the file data's hash as the
> > security.ima xattr, which is EVM protected. Other methods, such as
> > digital signatures, could be used instead of the file's hash, to
> > additionally provide authenticity.
>
> The problem is that your *design* assumes that the filesystem itself is
> valid, but your stated threat model assumes that the attacker has offline
> access to the filesystem, an explicit contradiction.
Agreed we did not describe our model clearly. But as stated in the prior
post, EVM's part of the defense in depth is to protect security xattrs
(not the data) from attack, and EVM itself is immune to these attacks,
since the attacker cannot forge an acceptable verifier.
> There have been numerous cases in the past where a corrupt or invalid
> filesystem causes kernel panics or even exploitable overflows or memory
> corruption; see the history of the "fsfuzzer" tool for more information.
Seems to me code bugs in the kernel should be fixed, given the universal
practice of automounting of removable media, and loopback mounting
images, regardless of EVM.
> Furthermore, if the attacker can intentionally cause data extent or inode
> extended attribute aliasing (shared space-on-disk) between different
> files then your entire security model falls flat.
As discussed in the prior post, your attacks do succeed on
IMA-Appraisal-HASH as a known tradeoff, but I don't see how they
can succeed on IMA-Appraisal-DigitalSignature or EVM or IMA attestation.
Even if the attacker has complete control of the xattr data, he
does not have the key needed to forge an HMAC acceptable to EVM.
> So if you assume the attacker has raw access to the underlying filesystem
> then you MUST authenticate *all* of the low-level filesystem data,
> including the "implicit" metadata of allocation tables, extents, etc.
I still don't see how this affects EVM or IMA attestation...
As you suggested earlier, you could protect all of the fs metadata
at the dm level, as a complement to filesystem level protections.
It would be interesting to see if such protections of all fs metadata
could be done efficiently within a filesystem itself, as Mimi suggested,
but I don't see the need.
thanks
dave
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
@ 2011-07-14 15:07 ` David Safford
0 siblings, 0 replies; 41+ messages in thread
From: David Safford @ 2011-07-14 15:07 UTC (permalink / raw)
To: Kyle Moffett
Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-fsdevel,
James Morris, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> >> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> >> > protect the integrity of a running system from unauthorized changes. When
> >> > these protections are not running, such as when booting a malicious OS,
> >> > mounting the disk under a different operating system, or physically moving
> >> > the disk to another system, an "offline" attack is free to read and write
> >> > file data/metadata.
> >> >
> >> > 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 the IMA-appraisal
> >> > patchset, integrity appraisal decisions. This patchset provides the framework
> >> > and an initial method to detect offline tampering of the security extended
> >> > attributes. The initial method 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'. Other methods of validating the integrity
> >> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
> >>
> >> Hmm, I'm not sure that this design actually provides the protection that
> >> you claim it does.
> >>
> >> Specifically, you don't actually protect the on-disk data-structures that
> >> are far more vulnerable to malicious modification than the actual *values*
> >> of the extended attributes themselves.
> >
> > True, EVM only protects the file metadata. The patch description says,
> >
> > While this patchset does authenticate the security xattrs, and
> > cryptographically binds them to the inode, coming extensions
> > will bind other directory and inode metadata for more complete
> > protection.
> >
> > It should have said, "bind other directory, inode data and inode
> > metadata."
> >
> > In particular, IMA-appraisal stores the file data's hash as the
> > security.ima xattr, which is EVM protected. Other methods, such as
> > digital signatures, could be used instead of the file's hash, to
> > additionally provide authenticity.
>
> The problem is that your *design* assumes that the filesystem itself is
> valid, but your stated threat model assumes that the attacker has offline
> access to the filesystem, an explicit contradiction.
Agreed we did not describe our model clearly. But as stated in the prior
post, EVM's part of the defense in depth is to protect security xattrs
(not the data) from attack, and EVM itself is immune to these attacks,
since the attacker cannot forge an acceptable verifier.
> There have been numerous cases in the past where a corrupt or invalid
> filesystem causes kernel panics or even exploitable overflows or memory
> corruption; see the history of the "fsfuzzer" tool for more information.
Seems to me code bugs in the kernel should be fixed, given the universal
practice of automounting of removable media, and loopback mounting
images, regardless of EVM.
> Furthermore, if the attacker can intentionally cause data extent or inode
> extended attribute aliasing (shared space-on-disk) between different
> files then your entire security model falls flat.
As discussed in the prior post, your attacks do succeed on
IMA-Appraisal-HASH as a known tradeoff, but I don't see how they
can succeed on IMA-Appraisal-DigitalSignature or EVM or IMA attestation.
Even if the attacker has complete control of the xattr data, he
does not have the key needed to forge an HMAC acceptable to EVM.
> So if you assume the attacker has raw access to the underlying filesystem
> then you MUST authenticate *all* of the low-level filesystem data,
> including the "implicit" metadata of allocation tables, extents, etc.
I still don't see how this affects EVM or IMA attestation...
As you suggested earlier, you could protect all of the fs metadata
at the dm level, as a complement to filesystem level protections.
It would be interesting to see if such protections of all fs metadata
could be done efficiently within a filesystem itself, as Mimi suggested,
but I don't see the need.
thanks
dave
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-07-14 15:07 ` David Safford
(?)
@ 2011-07-18 13:45 ` Serge E. Hallyn
-1 siblings, 0 replies; 41+ messages in thread
From: Serge E. Hallyn @ 2011-07-18 13:45 UTC (permalink / raw)
To: David Safford
Cc: Kyle Moffett, Mimi Zohar, linux-security-module, linux-kernel,
linux-fsdevel, James Morris, Andrew Morton, Greg KH,
Dmitry Kasatkin
Quoting David Safford (safford@watson.ibm.com):
> On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> > There have been numerous cases in the past where a corrupt or invalid
> > filesystem causes kernel panics or even exploitable overflows or memory
> > corruption; see the history of the "fsfuzzer" tool for more information.
>
> Seems to me code bugs in the kernel should be fixed, given the universal
> practice of automounting of removable media, and loopback mounting
> images, regardless of EVM.
Hi David,
yeah, this would also be nice for making people feel cozier about
supporting unprivileged fs mounts in general. I wonder if a real
project around the idea of strengthening the robustness of the fs
code, starting with the superblock parsing for a few of the most
comment filesystems, could take off. A combination of
. code auditing and test (i.e. fsfuzzer)
. moving parts of the code to unprivileged userspace
. marking audited filesystems as unprivileged-mountable, in the
way Miklos' patchset a few years ago did
. so that those who want to can refuse auto-mount of any not
audited filesystems.
-serge
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-29 20:53 ` Kyle Moffett
(?)
(?)
@ 2011-07-14 15:07 ` David Safford
-1 siblings, 0 replies; 41+ messages in thread
From: David Safford @ 2011-07-14 15:07 UTC (permalink / raw)
To: Kyle Moffett
Cc: Mimi Zohar, linux-security-module, linux-kernel, linux-fsdevel,
James Morris, Andrew Morton, Greg KH, Dmitry Kasatkin
On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> > protect the integrity of a running system from unauthorized changes. When
> > these protections are not running, such as when booting a malicious OS,
> > mounting the disk under a different operating system, or physically moving
> > the disk to another system, an "offline" attack is free to read and write
> > file data/metadata.
> >
> > 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 the IMA-appraisal
> > patchset, integrity appraisal decisions. This patchset provides the framework
> > and an initial method to detect offline tampering of the security extended
> > attributes. The initial method 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'. Other methods of validating the integrity
> > of a file's metadata will be posted separately (eg. EVM-digital-signatures).
>
> Hmm, I'm not sure that this design actually provides the protection that
> you claim it does.
My apologies for not responding sooner - I was on vacation when this
thread was discussed, and I wanted to take the time to make sure I
understood all of the ramifications of the attacks you raised.
Having studied the potential attacks based on fs metadata manipulation,
I believe that EVM (this particular patch set) is explicitly immune to the
attacks, and does provide exactly what it claims - protection of a file's
sensitive xattrs against offline attack.
EVM stores an HMAC or a Digital Signature across the designated set of
sensitive xattrs. The corresponding secret or private key is not known
to the attacker, and the attacker cannot therefore forge a valid
HMAC/signature. On xattr read, EVM verifies the value, so attacker
supplied data, even in the presence of offline or fs metadata
based attack, will not be accepted by EVM. On authorized update
of a protected xattr, EVM explicitly verifies the current xattr
data to make sure it has not been tampered in the meanwhile, and
updates the verifier only if the data has not been tampered. Without
the corresponding key, even if the attacker can tamper the xattrs,
he cannot do so in a way that EVM will accept.
You did correctly point out a vulnerability of one of the other integrity
modules (IMA-Appraisal-HASH) to these attacks, but this is a known tradeoff
with that particular module, which can be avoided/detected with other
modules (IMA-Appraisal-DigitalSignature, and IMA measurement/attestation.)
For unchanging files, IMA-Appraisal-DigitalSignature verifies the file's
data against a digital signature (typically from the Distro), and treats
the file as immutable, so even if an attacker changes the data through
an fs-metadata attack, the change will succeed while the inode
is cached, but it will not persist - it will be detected whenever
the inode is next reloaded from disk. Without the corresponding private
key, neither the attacker nor the kernel can update the verifying
signature.
For changing files, IMA-Appraisal-HASH does allow the file's hash
to be updated, and this does open the described attack, as it
cannot distinguish a good change in the file's hash from a malicious
one induced by the fs-metadata attack outlined. (And IMA-Appraisal-HASH
is certainly effective for other non-offline attacks.)
However any such change, once persistent, will be detected by the base
IMA measurement and attestation. Regardless of how a file is changed,
any persistent file change will be detected by IMA attestation,
because it's measurement chain is rooted in hardware, and it cannot
be forged by software.
So the overall integrity architecture provides defense in depth;
MAC (selinux, apparmor, smack...) will try to confine the damage
from a data exploited program, IMA-Appraisal will try to detect
and block damage locally, EVM will protect xattrs from damage,
and IMA attestation is the final remote catcher of anything that
makes it through all of these layers.
> Specifically, you don't actually protect the on-disk data-structures that
> are far more vulnerable to malicious modification than the actual *values*
> of the extended attributes themselves.
>
> For example, compare the number of actual practical exploits of image
> buffer-overflow vulnerabilities involving user-generated content versus
> the number of Man-in-the-middle exploits of trusted content, I guarantee
> you there are a lot more of the former in the wild.
Yes, but most data driven attacks of vulnerable software do try to become
persistent, at which point they become detectable. Even the Morris worm
wrote out one file.
> It's also worth mentioning that one of the reasons we have traditionally
> prevented user mounts of many filesystems is because the filesystem
> drivers tended to be buggy and susceptible to panic or buffer overflow.
Every system I have used (fedora, ubuntu, android...) by default
allows automatic mounting of removable media, and fs images. That seems to
me to be a problem if there is exploitable fs code.
> To give you a trivial example of how this could be subverted, you could
> modify the block mapping table of an unprotected file to reference some
> blocks used in a "protected" file. Then boot up the system and "verify"
> the protected file, and then write new data to the unprotected file and
> generate some memory pressure to force the protected data to be reloaded
> from disk.
As discussed earlier, this example is an attack on IMA-Appraisal-HASH, not
IMA-Appraisal-DigitalSignature (which treats such files as immutable), or
on EVM at all, and even then it will be detected by IMA attestation.
> Another good example of how this breaks down in practice would be the
> various incompatibilities in different VFAT filesystem behavior with
> NUL filenames. I don't remember what the change was on Linux, but in
> practice on several operating systems you can "mkdir" a new directory
> entry and several more could suddenly appear (because the NUL entry is
> now populated).
I haven't looked in detail at this, but I believe the IMA-Appraisal-directory
extension will handle this, and it does not affect EVM itself.
> If you want to use HMAC verification of disk contents then you should not
> be performing the HMAC of some partial logical view of them, but instead
> protect the *disk contents* themselves!!!
>
> Perhaps modifying dm-crypt to support HMAC-based authentication modes
> would be one approach. You could probably even code it to store the HMAC
> metadata in a separate partition to allow for read-only access by your
> bootloader, etc.
There are big tradeoffs doing integrity at the disk vs filesystem level.
For example, if we do integrity checking only at the dm level, then
we cannot have policy based verification of digitally signed files. This
is a very important use case for some customers.
thanks,
dave
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
@ 2011-06-30 21:06 ` Ryan Ware
2011-06-29 19:50 ` [PATCH v7 02/16] integrity: move ima inode integrity data management Mimi Zohar
` (17 subsequent siblings)
18 siblings, 0 replies; 41+ messages in thread
From: Ryan Ware @ 2011-06-30 21:06 UTC (permalink / raw)
To: Mimi Zohar, linux-security-module
Cc: linux-kernel, linux-fsdevel, James Morris, David Safford,
Andrew Morton, Greg KH, Dmitry Kasatkin
Glad to see this going in Mimi! Looking forward to enabling this in our
MeeGo kernels.
Ryan
On 6/29/11 12:50 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
>Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
>protect the integrity of a running system from unauthorized changes. When
>these protections are not running, such as when booting a malicious OS,
>mounting the disk under a different operating system, or physically moving
>the disk to another system, an "offline" attack is free to read and write
>file data/metadata.
>
>...snip...
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
@ 2011-06-30 21:06 ` Ryan Ware
0 siblings, 0 replies; 41+ messages in thread
From: Ryan Ware @ 2011-06-30 21:06 UTC (permalink / raw)
To: Mimi Zohar, linux-security-module
Cc: linux-kernel, linux-fsdevel, James Morris, David Safford,
Andrew Morton, Greg KH, Dmitry Kasatkin
Glad to see this going in Mimi! Looking forward to enabling this in our
MeeGo kernels.
Ryan
On 6/29/11 12:50 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
>Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
>protect the integrity of a running system from unauthorized changes. When
>these protections are not running, such as when booting a malicious OS,
>mounting the disk under a different operating system, or physically moving
>the disk to another system, an "offline" attack is free to read and write
>file data/metadata.
>
>...snip...
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-30 21:06 ` Ryan Ware
@ 2011-06-30 22:37 ` Mimi Zohar
-1 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-30 22:37 UTC (permalink / raw)
To: Ryan Ware
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Thu, 2011-06-30 at 14:06 -0700, Ryan Ware wrote:
> Glad to see this going in Mimi! Looking forward to enabling this in our
> MeeGo kernels.
>
> Ryan
I wish. As far as I'm aware, EVM hasn't been upstreamed. The good news
is that the ecryptfs encrypted-key patches are now in the
security-testing tree.
thanks,
Mimi
> On 6/29/11 12:50 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
>
> >Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> >protect the integrity of a running system from unauthorized changes. When
> >these protections are not running, such as when booting a malicious OS,
> >mounting the disk under a different operating system, or physically moving
> >the disk to another system, an "offline" attack is free to read and write
> >file data/metadata.
> >
> >...snip...
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
@ 2011-06-30 22:37 ` Mimi Zohar
0 siblings, 0 replies; 41+ messages in thread
From: Mimi Zohar @ 2011-06-30 22:37 UTC (permalink / raw)
To: Ryan Ware
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Thu, 2011-06-30 at 14:06 -0700, Ryan Ware wrote:
> Glad to see this going in Mimi! Looking forward to enabling this in our
> MeeGo kernels.
>
> Ryan
I wish. As far as I'm aware, EVM hasn't been upstreamed. The good news
is that the ecryptfs encrypted-key patches are now in the
security-testing tree.
thanks,
Mimi
> On 6/29/11 12:50 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
>
> >Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> >protect the integrity of a running system from unauthorized changes. When
> >these protections are not running, such as when booting a malicious OS,
> >mounting the disk under a different operating system, or physically moving
> >the disk to another system, an "offline" attack is free to read and write
> >file data/metadata.
> >
> >...snip...
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-30 22:37 ` Mimi Zohar
@ 2011-07-01 2:02 ` Ware, Ryan R
-1 siblings, 0 replies; 41+ messages in thread
From: Ware, Ryan R @ 2011-07-01 2:02 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Thu, Jun 30, 2011 at 3:37 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>
> On Thu, 2011-06-30 at 14:06 -0700, Ryan Ware wrote:
> > Glad to see this going in Mimi! Looking forward to enabling this in our
> > MeeGo kernels.
> >
> > Ryan
>
> I wish. As far as I'm aware, EVM hasn't been upstreamed. The good news
> is that the ecryptfs encrypted-key patches are now in the
> security-testing tree.
True. My wording should have been clearer. It's definitely good to
have the ecryptfs patches in.
>
> > On 6/29/11 12:50 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
> >
> > >Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> > >protect the integrity of a running system from unauthorized changes. When
> > >these protections are not running, such as when booting a malicious OS,
> > >mounting the disk under a different operating system, or physically moving
> > >the disk to another system, an "offline" attack is free to read and write
> > >file data/metadata.
> > >
> > >...snip...
> >
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
@ 2011-07-01 2:02 ` Ware, Ryan R
0 siblings, 0 replies; 41+ messages in thread
From: Ware, Ryan R @ 2011-07-01 2:02 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel, James Morris,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Thu, Jun 30, 2011 at 3:37 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>
> On Thu, 2011-06-30 at 14:06 -0700, Ryan Ware wrote:
> > Glad to see this going in Mimi! Looking forward to enabling this in our
> > MeeGo kernels.
> >
> > Ryan
>
> I wish. As far as I'm aware, EVM hasn't been upstreamed. The good news
> is that the ecryptfs encrypted-key patches are now in the
> security-testing tree.
True. My wording should have been clearer. It's definitely good to
have the ecryptfs patches in.
>
> > On 6/29/11 12:50 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote:
> >
> > >Discretionary Access Control(DAC) and Mandatory Access Control(MAC) can
> > >protect the integrity of a running system from unauthorized changes. When
> > >these protections are not running, such as when booting a malicious OS,
> > >mounting the disk under a different operating system, or physically moving
> > >the disk to another system, an "offline" attack is free to read and write
> > >file data/metadata.
> > >
> > >...snip...
> >
> >
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v7 00/16] EVM
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
` (17 preceding siblings ...)
2011-06-30 21:06 ` Ryan Ware
@ 2011-07-18 23:52 ` James Morris
2011-07-19 20:56 ` Mimi Zohar
18 siblings, 1 reply; 41+ messages in thread
From: James Morris @ 2011-07-18 23:52 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
The discussion on this seems to have run its course.
Does anyone have any further issues with these patches?
Otherwise, I will merge EVM.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
2011-07-18 23:52 ` James Morris
@ 2011-07-19 20:56 ` Mimi Zohar
2011-08-09 1:53 ` James Morris
0 siblings, 1 reply; 41+ messages in thread
From: Mimi Zohar @ 2011-07-19 20:56 UTC (permalink / raw)
To: James Morris
Cc: linux-security-module, linux-kernel, linux-fsdevel,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Tue, 2011-07-19 at 09:52 +1000, James Morris wrote:
> The discussion on this seems to have run its course.
>
> Does anyone have any further issues with these patches?
>
> Otherwise, I will merge EVM.
>
>
> - James
James,
If/when you're ready to merge the EVM patches, they have been rebased
against the latest security-testing-2.6/#next and a couple of
whitespaces removed. No other changes were made.
The following changes since commit 0f2a55d5bb2372058275b0b343d90dd5d640d045:
TOMOYO: Update kernel-doc. (2011-07-14 17:50:03 +1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/ima-2.6.git next-evm
Dmitry Kasatkin (5):
evm: add support for different security.evm data types
evm: crypto hash replaced by shash
evm: additional parameter to pass integrity cache entry 'iint'
evm: evm_verify_hmac must not return INTEGRITY_UNKNOWN
evm: replace hmac_status with evm_status
Mimi Zohar (11):
security: new security_inode_init_security API adds function callback
integrity: move ima inode integrity data management
xattr: define vfs_getxattr_alloc and vfs_xattr_cmp
evm: re-release
security: imbed evm calls in security hooks
evm: evm_inode_post_removexattr
evm: imbed evm_inode_post_setattr
evm: add evm_inode_init_security to initialize new files
evm: call evm_inode_init_security from security_inode_init_security
evm: permit only valid security.evm xattrs to be updated
evm: add evm_inode_setattr to prevent updating an invalid security.evm
thanks,
Mimi
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v7 00/16] EVM
2011-07-19 20:56 ` Mimi Zohar
@ 2011-08-09 1:53 ` James Morris
0 siblings, 0 replies; 41+ messages in thread
From: James Morris @ 2011-08-09 1:53 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-fsdevel,
David Safford, Andrew Morton, Greg KH, Dmitry Kasatkin
On Tue, 19 Jul 2011, Mimi Zohar wrote:
> If/when you're ready to merge the EVM patches, they have been rebased
> against the latest security-testing-2.6/#next and a couple of
> whitespaces removed. No other changes were made.
>
> The following changes since commit 0f2a55d5bb2372058275b0b343d90dd5d640d045:
Merged with a couple of fixups into:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 41+ messages in thread