2011-06-29 19:51:26

by Mimi Zohar

Subject: [PATCH v7 00/16] EVM

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

Although an offline attack can bypass DAC/MAC protection mechanisms and write
file data/metadata, if the disk, or VM, is subsequently remounted under the
EVM + DAC/MAC (+ IMA-appraisal) protected OS, then the TPM-calculated HMAC of
the file's metadata won't be valid. Therefore, IMA + MAC/DAC + EVM
(+ IMA-appraisal) can protect system integrity online, detect offline tampering,
and prevent tampered files from being accessed.

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-digital-signatures (including module checking),
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.

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

Changes from v6:
- Changed the security_inode_init_security API to write the security xattr,
by calling an fs specific callback.
- Moved the evm_inode_post_init() calls, which calculate the EVM xattr, from
each fs to security_inode_init_security.
- Renamed evm_inode_post_init_security() to evm_inode_init_security().
- Renamed the boot parameter evm_mode=' to 'evm='.

Changes from v5:
- defined 'struct evm_ima_xattr_data',
removed MAX_DIGEST_SIZE and evm_hmac_size definitions
- check for key failures and errors earlier
- other minor changes enumerated in individual patch descriptions

Changes from v4:
- Added evm_inode_post_init calls for: btrfs, gfs2, jffs2, jfs, and xfs.
- Prevent an invalid security.evm xattr from being updated.
- evm_verifyxattr() performance improvement (Dmitry Kasatkin)
- Fixed evm_verify_hmac() to be fail safe (Dmitry Kasatkin)
- Additional naming change generalizations in preparation for other methods
of integrity authentication. (Dmitry Kasatkin)

Mimi Zohar
David Safford

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

Documentation/ABI/testing/evm | 23 ++
Documentation/kernel-parameters.txt | 6 +
fs/attr.c | 5 +-
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/xattr.c | 63 ++++++-
fs/xfs/linux-2.6/xfs_iops.c | 39 ++--
include/linux/evm.h | 92 +++++++++
include/linux/ima.h | 13 --
include/linux/integrity.h | 38 ++++
include/linux/security.h | 17 +-
include/linux/xattr.h | 14 ++-
mm/shmem.c | 4 +-
security/Kconfig | 2 +-
security/Makefile | 4 +-
security/integrity/Kconfig | 7 +
security/integrity/Makefile | 12 +
security/integrity/evm/Kconfig | 12 +
security/integrity/evm/Makefile | 6 +
security/integrity/evm/evm.h | 38 ++++
security/integrity/evm/evm_crypto.c | 216 ++++++++++++++++++++
security/integrity/evm/evm_main.c | 384 +++++++++++++++++++++++++++++++++++
security/integrity/evm/evm_secfs.c | 108 ++++++++++
security/integrity/iint.c | 171 ++++++++++++++++
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 | 47 +++++
security/security.c | 71 ++++++-
39 files changed, 1537 insertions(+), 407 deletions(-)
create mode 100644 Documentation/ABI/testing/evm
create mode 100644 include/linux/evm.h
create mode 100644 include/linux/integrity.h
create mode 100644 security/integrity/Kconfig
create mode 100644 security/integrity/Makefile
create mode 100644 security/integrity/evm/Kconfig
create mode 100644 security/integrity/evm/Makefile
create mode 100644 security/integrity/evm/evm.h
create mode 100644 security/integrity/evm/evm_crypto.c
create mode 100644 security/integrity/evm/evm_main.c
create mode 100644 security/integrity/evm/evm_secfs.c
create mode 100644 security/integrity/iint.c
delete mode 100644 security/integrity/ima/ima_iint.c
create mode 100644 security/integrity/integrity.h

--
1.7.3.4

2011-06-29 19:51:52

by Mimi Zohar

Subject: [PATCH v7 01/16] security: new security_inode_init_security API adds function callback

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 <[email protected]>
---
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

2011-06-29 19:52:11

by Mimi Zohar

Subject: [PATCH v7 02/16] integrity: move ima inode integrity data management

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 <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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 <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#ifndef _LINUX_INTEGRITY_H
+#define _LINUX_INTEGRITY_H
+
+#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 <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: integrity_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 <[email protected]>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation, version 2 of the
- * License.
- *
- * File: ima_iint.c
- * - implements the IMA hooks: ima_inode_alloc, ima_inode_free
- * - cache integrity information associated with an inode
- * using a 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 <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+
+#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

2011-06-29 19:52:14

by Mimi Zohar

Subject: [PATCH v7 03/16] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp

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

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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

2011-06-29 19:52:00

by Mimi Zohar

Subject: [PATCH v7 04/16] evm: re-release

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 <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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 <[email protected]>
+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 <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm.h
+ *
+ */
+#include <linux/security.h>
+#include "../integrity.h"
+
+extern int evm_initialized;
+extern char *evm_hmac;
+
+/* 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 <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm_crypto.c
+ * Using root's kernel master key (kmk), calculate the HMAC
+ */
+
+#include <linux/module.h>
+#include <linux/crypto.h>
+#include <linux/xattr.h>
+#include <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 <[email protected]>
+ * Kylene Hall <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm_main.c
+ * implements evm_inode_setxattr, evm_inode_post_setxattr,
+ * evm_inode_removexattr, and evm_verifyxattr
+ */
+
+#include <linux/module.h>
+#include <linux/crypto.h>
+#include <linux/xattr.h>
+#include <linux/integrity.h>
+#include "evm.h"
+
+int evm_initialized;
+
+char *evm_hmac = "hmac(sha1)";
+
+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 <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * File: evm_secfs.c
+ * - Used to signal when key is on keyring
+ * - Get the key and enable EVM
+ */
+
+#include <linux/uaccess.h>
+#include <linux/module.h>
+#include "evm.h"
+
+static struct dentry *evm_init_tpm;
+
+/**
+ * evm_read_key - read() for <securityfs>/evm
+ *
+ * @filp: file pointer, not actually used
+ * @buf: where to put the result
+ * @count: maximum to send along
+ * @ppos: where to start
+ *
+ * Returns number of bytes read or error code, as appropriate
+ */
+static ssize_t evm_read_key(struct file *filp, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ char temp[80];
+ ssize_t rc;
+
+ if (*ppos != 0)
+ return 0;
+
+ sprintf(temp, "%d", evm_initialized);
+ rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+
+ return rc;
+}
+
+/**
+ * evm_write_key - write() for <securityfs>/evm
+ * @file: file pointer, not actually used
+ * @buf: where to get the data from
+ * @count: bytes sent
+ * @ppos: where to start
+ *
+ * Used to signal that key is on the kernel key ring.
+ * - get the integrity hmac key from the kernel key ring
+ * - create list of hmac protected extended attributes
+ * Returns number of bytes written or error code, as appropriate
+ */
+static ssize_t evm_write_key(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ char temp[80];
+ int i, error;
+
+ if (!capable(CAP_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

2011-06-29 19:51:56

by Mimi Zohar

Subject: [PATCH v7 05/16] evm: add support for different security.evm data types

From: Dmitry Kasatkin <[email protected]>

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 <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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

2011-06-29 19:52:32

by Mimi Zohar

Subject: [PATCH v7 06/16] security: imbed evm calls in security hooks

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

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

Changelog:
- Don't define evm_verifyxattr(), unless CONFIG_INTEGRITY is enabled.
- xattr_name is a 'const', value is 'void *'

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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 <[email protected]>
+ */
+
+#ifndef _LINUX_EVM_H
+#define _LINUX_EVM_H
+
+#include <linux/integrity.h>
+
+#ifdef CONFIG_EVM
+extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
+ 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

2011-06-29 19:52:29

by Mimi Zohar

Subject: [PATCH v7 07/16] evm: evm_inode_post_removexattr

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

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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

2011-06-29 19:52:46

by Mimi Zohar

Subject: [PATCH v7 08/16] evm: imbed evm_inode_post_setattr

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

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
---
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

2011-06-29 19:53:33

by Mimi Zohar

Subject: [PATCH v7 09/16] evm: add evm_inode_init_security to initialize new files

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 <[email protected]>
---
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

2011-06-29 19:52:41

by Mimi Zohar

Subject: [PATCH v7 10/16] evm: call evm_inode_init_security from security_inode_init_security

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 <[email protected]>
---
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

2011-06-29 19:54:35

by Mimi Zohar

Subject: [PATCH v7 11/16] evm: crypto hash replaced by shash

From: Dmitry Kasatkin <[email protected]>

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 <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
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

2011-06-29 19:53:00

by Mimi Zohar

Subject: [PATCH v7 12/16] evm: additional parameter to pass integrity cache entry 'iint'

From: Dmitry Kasatkin <[email protected]>

Additional iint parameter allows to skip lookup in the cache.

Signed-off-by: Dmitry Kasatkin <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
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

2011-06-29 19:52:49

by Mimi Zohar

Subject: [PATCH v7 13/16] evm: evm_verify_hmac must not return INTEGRITY_UNKNOWN

From: Dmitry Kasatkin <[email protected]>

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 <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
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

2011-06-29 19:53:04

by Mimi Zohar

Subject: [PATCH v7 14/16] evm: replace hmac_status with evm_status

From: Dmitry Kasatkin <[email protected]>

We will use digital signatures in addtion to hmac.

Signed-off-by: Dmitry Kasatkin <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
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

2011-06-29 19:52:55

by Mimi Zohar

Subject: [PATCH v7 15/16] evm: permit only valid security.evm xattrs to be updated

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 <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
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

2011-06-29 19:54:00

by Mimi Zohar

Subject: [PATCH v7 16/16] evm: add evm_inode_setattr to prevent updating an invalid security.evm

Permit changing of security.evm only when valid, unless in fixmode.

Reported-by: Roberto Sassu <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
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

2011-06-29 20:53:48

by Kyle Moffett

Subject: Re: [PATCH v7 00/16] EVM

On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <[email protected]> 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

2011-06-29 23:42:40

by Mimi Zohar

Subject: Re: [PATCH v7 00/16] EVM

On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <[email protected]> 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

2011-06-30 01:58:10

by Kyle Moffett

Subject: Re: [PATCH v7 00/16] EVM

On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <[email protected]> wrote:
> On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
>> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <[email protected]> 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

2011-06-30 03:51:54

by Mimi Zohar

Subject: Re: [PATCH v7 00/16] EVM

On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <[email protected]> wrote:
> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> >> On Wed, Jun 29, 2011 at 15:50, Mimi Zohar <[email protected]> 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

2011-06-30 21:06:44

by Ryan Ware

Subject: Re: [PATCH v7 00/16] EVM

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" <[email protected]> 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...

2011-06-30 22:33:21

by Kyle Moffett

Subject: Re: [PATCH v7 00/16] EVM

Whoops, resent in plain text, sorry about the HTML

On Wed, Jun 29, 2011 at 23:51, Mimi Zohar <[email protected]> wrote:
> On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
>> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <[email protected]> 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

2011-06-30 22:37:57

by Mimi Zohar

Subject: Re: [PATCH v7 00/16] EVM

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" <[email protected]> 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...
>
>

2011-07-01 02:02:33

by Ware, Ryan R

Subject: Re: [PATCH v7 00/16] EVM

On Thu, Jun 30, 2011 at 3:37 PM, Mimi Zohar <[email protected]> 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" <[email protected]> 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/