2008-08-08 18:56:27

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

This version resolves the merge issues resulting from the removal of
the nameidata parameter to inode_permission().
- Moved the integrity_inode_permission() call from inode_permission()
to vfs_inode_permission() and file_inode_permission().
- Replaced the inode and nameidata parameters with file and path
parameters to integrity_inode_permission().

This patch is a redesign of the integrity framework, which address a
number of issues, including
- generalizing the measurement API beyond just inode measurements.
- separation of the measurement into distinct collection, appraisal,
and commitment phases, for greater flexibility.

Extended Verification Module(EVM) and the Integrity Measurement
Architecture(IMA) were originally implemented as an LSM module. Based
on discussions on the LSM mailing list, a decision was made that the
LSM hooks should only be used to enforce mandatory access control
decisions and a new set of hooks should be defined specifically for
integrity.

EVM/IMA was limited to verifying and measuring a file's (i.e. an inode)
integrity and the metadata associated with it. Current research is
looking into other types of integrity measurements. (i.e. "Linux kernel
integrity measurement using contextual inspection", by Peter A. Loscocco,
Perry W. Wilson, J. Aaron Pendergrass, C. Durward McDonell,
http://doi.acm.org/10.1145/1314354.1314362). As a result, a requirement
of the new integrity framework is support for different types of integrity
measurements.

This patch provides an integrity framework(api and hooks) and placement
of the integrity hooks in the appropriate places in the fs directory.
Collecting, appraising, and storing of file and other types of integrity
data is supported. Multiple integrity templates, which implement the
integrity API, may register themselves. For now, only a single integrity
provider can register itself for the integrity hooks. (Support for multiple
providers registering themselves for the integrity hooks would require
some form of stacking.)

The six integrity hooks are:
inode_permission, inode_alloc_integrity, inode_free_integrity,
bprm_check_integrity, file_free_integrity, file_mmap

The five integrity API calls provided are:
integrity_must_measure, integrity_collect_measurement,
integrity_appraise_measurement, integrity_store_measurement,
and integrity_display_template.

The type of integrity data being collected, appraised, stored, or
displayed is template dependent.

(Details on the calls and their exact arguments are in linux/integrity.h,
included in the patch.)

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

Index: security-testing-2.6/Documentation/kernel-parameters.txt
===================================================================
--- security-testing-2.6.orig/Documentation/kernel-parameters.txt
+++ security-testing-2.6/Documentation/kernel-parameters.txt
@@ -44,6 +44,7 @@ parameter is applicable:
FB The frame buffer device is enabled.
HW Appropriate hardware is enabled.
IA-64 IA-64 architecture is enabled.
+ INTEGRITY Integrity support is enabled.
IOSCHED More than one I/O scheduler is enabled.
IP_PNP IP DHCP, BOOTP, or RARP is enabled.
ISAPNP ISA PnP code is enabled.
@@ -878,6 +879,11 @@ and is between 256 and 4096 characters.
inport.irq= [HW] Inport (ATI XL and Microsoft) busmouse driver
Format: <irq>

+ integrity_audit= [INTEGRITY]
+ Format: { "0" | "1" }
+ 0 -- disable integrity auditing messages.
+ 1 -- enable integrity auditing messages. (Default)
+
inttest= [IA64]

iommu= [x86]
Index: security-testing-2.6/fs/exec.c
===================================================================
--- security-testing-2.6.orig/fs/exec.c
+++ security-testing-2.6/fs/exec.c
@@ -45,6 +45,7 @@
#include <linux/proc_fs.h>
#include <linux/mount.h>
#include <linux/security.h>
+#include <linux/integrity.h>
#include <linux/syscalls.h>
#include <linux/tsacct_kern.h>
#include <linux/cn_proc.h>
@@ -1204,6 +1205,9 @@ int search_binary_handler(struct linux_b
retval = security_bprm_check(bprm);
if (retval)
return retval;
+ retval = integrity_bprm_check(bprm);
+ if (retval)
+ return retval;

/* kernel module loader fixup */
/* so we don't try to load run modprobe in kernel space. */
Index: security-testing-2.6/fs/file_table.c
===================================================================
--- security-testing-2.6.orig/fs/file_table.c
+++ security-testing-2.6/fs/file_table.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/security.h>
+#include <linux/integrity.h>
#include <linux/eventpoll.h>
#include <linux/rcupdate.h>
#include <linux/mount.h>
@@ -272,6 +273,7 @@ void __fput(struct file *file)
if (file->f_op && file->f_op->release)
file->f_op->release(inode, file);
security_file_free(file);
+ integrity_file_free(file);
if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL))
cdev_put(inode->i_cdev);
fops_put(file->f_op);
@@ -343,6 +345,7 @@ void put_filp(struct file *file)
{
if (atomic_long_dec_and_test(&file->f_count)) {
security_file_free(file);
+ integrity_file_free(file);
file_kill(file);
file_free(file);
}
Index: security-testing-2.6/fs/inode.c
===================================================================
--- security-testing-2.6.orig/fs/inode.c
+++ security-testing-2.6/fs/inode.c
@@ -17,6 +17,7 @@
#include <linux/hash.h>
#include <linux/swap.h>
#include <linux/security.h>
+#include <linux/integrity.h>
#include <linux/pagemap.h>
#include <linux/cdev.h>
#include <linux/bootmem.h>
@@ -151,6 +152,15 @@ static struct inode *alloc_inode(struct
return NULL;
}

+ /* allocate, attach and initialize an i_integrity */
+ if (integrity_inode_alloc(inode)) {
+ if (inode->i_sb->s_op->destroy_inode)
+ inode->i_sb->s_op->destroy_inode(inode);
+ else
+ kmem_cache_free(inode_cachep, (inode));
+ return NULL;
+ }
+
spin_lock_init(&inode->i_lock);
lockdep_set_class(&inode->i_lock, &sb->s_type->i_lock_key);

@@ -190,6 +200,7 @@ void destroy_inode(struct inode *inode)
{
BUG_ON(inode_has_buffers(inode));
security_inode_free(inode);
+ integrity_inode_free(inode);
if (inode->i_sb->s_op->destroy_inode)
inode->i_sb->s_op->destroy_inode(inode);
else
Index: security-testing-2.6/fs/namei.c
===================================================================
--- security-testing-2.6.orig/fs/namei.c
+++ security-testing-2.6/fs/namei.c
@@ -24,6 +24,7 @@
#include <linux/fsnotify.h>
#include <linux/personality.h>
#include <linux/security.h>
+#include <linux/integrity.h>
#include <linux/syscalls.h>
#include <linux/mount.h>
#include <linux/audit.h>
@@ -289,7 +290,14 @@ int inode_permission(struct inode *inode
*/
int vfs_permission(struct nameidata *nd, int mask)
{
- return inode_permission(nd->path.dentry->d_inode, mask);
+ int retval;
+
+ retval = inode_permission(nd->path.dentry->d_inode, mask);
+ if (retval)
+ return retval;
+ return integrity_inode_permission(NULL, &nd->path,
+ mask & (MAY_READ | MAY_WRITE |
+ MAY_EXEC));
}

/**
@@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
*/
int file_permission(struct file *file, int mask)
{
- return inode_permission(file->f_path.dentry->d_inode, mask);
+ int retval;
+
+ retval = inode_permission(file->f_path.dentry->d_inode, mask);
+ if (retval)
+ return retval;
+ return integrity_inode_permission(file, NULL,
+ mask & (MAY_READ | MAY_WRITE |
+ MAY_EXEC));
}

/*
Index: security-testing-2.6/include/linux/audit.h
===================================================================
--- security-testing-2.6.orig/include/linux/audit.h
+++ security-testing-2.6/include/linux/audit.h
@@ -123,6 +123,11 @@
#define AUDIT_LAST_KERN_ANOM_MSG 1799
#define AUDIT_ANOM_PROMISCUOUS 1700 /* Device changed promiscuous mode */
#define AUDIT_ANOM_ABEND 1701 /* Process ended abnormally */
+#define AUDIT_INTEGRITY_DATA 1800 /* Data integrity verification */
+#define AUDIT_INTEGRITY_METADATA 1801 /* Metadata integrity verification */
+#define AUDIT_INTEGRITY_STATUS 1802 /* Integrity enable status */
+#define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */
+#define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */

#define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */

@@ -440,6 +445,8 @@ extern int audit_set_loginuid(struct ta
#define audit_get_loginuid(t) ((t)->loginuid)
#define audit_get_sessionid(t) ((t)->sessionid)
extern void audit_log_task_context(struct audit_buffer *ab);
+extern void audit_log_inode_context(struct audit_buffer *ab,
+ struct inode *inode);
extern int __audit_ipc_obj(struct kern_ipc_perm *ipcp);
extern int __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, mode_t mode);
extern int audit_bprm(struct linux_binprm *bprm);
@@ -520,6 +527,7 @@ extern int audit_signals;
#define audit_get_loginuid(t) (-1)
#define audit_get_sessionid(t) (-1)
#define audit_log_task_context(b) do { ; } while (0)
+#define audit_log_inode_context(b, a) do { } while (0)
#define audit_ipc_obj(i) ({ 0; })
#define audit_ipc_set_perm(q,u,g,m) ({ 0; })
#define audit_bprm(p) ({ 0; })
Index: security-testing-2.6/include/linux/fs.h
===================================================================
--- security-testing-2.6.orig/include/linux/fs.h
+++ security-testing-2.6/include/linux/fs.h
@@ -680,6 +680,9 @@ struct inode {
#ifdef CONFIG_SECURITY
void *i_security;
#endif
+#ifdef CONFIG_INTEGRITY
+ void *i_integrity;
+#endif
void *i_private; /* fs or device private pointer */
};

Index: security-testing-2.6/include/linux/integrity.h
===================================================================
--- /dev/null
+++ security-testing-2.6/include/linux/integrity.h
@@ -0,0 +1,184 @@
+/*
+ * integrity.h
+ *
+ * Copyright (C) 2005,2006,2007,2008 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>
+#include <linux/audit.h>
+
+#ifdef CONFIG_INTEGRITY
+void integrity_audit_msg(int audit_msgno, struct inode *inode,
+ const unsigned char *fname, char *op,
+ char *cause, int result);
+
+/*
+ * Integrity API calls:
+ *
+ * @collect_measurement:
+ * Collect template specific measurement data.
+ * @data contains template specific data used for collecting the
+ * measurement.
+ * Return 0 if operation was successful.
+ *
+ * @appraise_measurement:
+ * Appraise the integrity of the template specific measurement data.
+ * @data contains template specific data used for appraising the
+ * measurement.
+ * Return 0 if operation was successful.
+ *
+ * @store_measurement:
+ * Store the template specific data.
+ * @data contains template specific data used for storing the
+ * measurement.
+ *
+ * @must_measure:
+ * Measurement decision based on an integrity policy.
+ * @data contains template specific data used for making policy
+ * decision.
+ * Return 0 if operation was successful.
+ *
+ * @display_template:
+ * Display template specific data.
+ *
+ */
+
+enum integrity_show_type { INTEGRITY_SHOW_BINARY, INTEGRITY_SHOW_ASCII};
+
+struct template_operations {
+ int (*collect_measurement)(void *);
+ int (*appraise_measurement)(void *);
+ void (*store_measurement)(void *);
+ int (*must_measure)(void *);
+ void (*display_template)(struct seq_file *m, void *,
+ enum integrity_show_type);
+};
+extern int integrity_register_template(const char *template_name,
+ const struct template_operations *ops);
+extern int integrity_unregister_template(const char *template_name);
+extern int integrity_find_template(const char *,
+ const struct template_operations **ops);
+
+/*
+ * Integrity hooks:
+ *
+ * @bprm_check_integrity:
+ * This hook mediates the point when a search for a binary handler will
+ * begin. At this point, the OS protects against an executable file,
+ * already open for write, from being executed; and an executable file
+ * already open for execute, from being modified. So we can be certain
+ * that any measurements(collect, appraise, store) done here are of
+ * the file being executed.
+ * @bprm contains the linux_binprm structure.
+ * Return 0 if the hook is successful and permission is granted.
+ *
+ * @inode_alloc_integrity:
+ * Allocate and attach an integrity structure to @inode->i_integrity. The
+ * i_integrity field is initialized to NULL when the inode structure is
+ * allocated.
+ * @inode contains the inode structure.
+ * Return 0 if operation was successful.
+ *
+ * @inode_free_integrity:
+ * @inode contains the inode structure.
+ * Deallocate the inode integrity structure and set @inode->i_integrity to
+ * NULL.
+ *
+ * @inode_permission:
+ * This hook is called by the existing Linux vfs_permission and
+ * file_permission functions, as a file is opened. At this point,
+ * measurements(collect, appraise, store) of files open for read
+ * can be made.
+ * @file contains the file structure of the file being opened(may be NULL).
+ * @path contains the path structure (may be NULL).
+ * @mask contains the permission mask.
+ * Return 0 if the hook is successful and permission is granted.
+ *
+ * @file_free_integrity:
+ * Update the integrity xattr value as necessary.
+ * @file contains the file structure being closed.
+ *
+ * @file_mmap :
+ * Measurements(collect, appraise, store) of files mmaped for EXEC
+ * can be made.
+ * @file contains the file structure of the file to map (may be NULL).
+ * @prot contains the protection that will be applied by the kernel.
+ * Return 0 if the hook is successful and permission is granted.
+ */
+
+enum lim_hooks {INODE_PERMISSION = 1, FILE_MMAP, BPRM_CHECK };
+
+struct integrity_operations {
+ int (*bprm_check_integrity) (struct linux_binprm *bprm);
+ int (*inode_alloc_integrity) (struct inode *inode);
+ void (*inode_free_integrity) (struct inode *inode);
+ int (*inode_permission) (struct file *file, struct path *path,
+ int mask);
+ void (*file_free_integrity) (struct file *file);
+ int (*file_mmap) (struct file *file, unsigned long prot);
+};
+extern int register_integrity(const struct integrity_operations *ops);
+extern int unregister_integrity(const struct integrity_operations *ops);
+
+/* global variables */
+extern const struct integrity_operations *integrity_ops;
+
+
+int integrity_collect_measurement(const char *template_name, void *data);
+int integrity_appraise_measurement(const char *template_name, void *data);
+int integrity_must_measure(const char *template_name, void *data);
+int integrity_store_measurement(const char *template_name, void *data);
+
+int integrity_bprm_check(struct linux_binprm *bprm);
+int integrity_inode_alloc(struct inode *inode);
+void integrity_inode_free(struct inode *inode);
+int integrity_inode_permission(struct file *file, struct path *path,int mask);
+void integrity_file_free(struct file *file);
+int integrity_file_mmap(struct file *file, unsigned long prot);
+#else
+
+static inline int integrity_bprm_check(struct linux_binprm *bprm)
+{
+ return 0;
+}
+
+static inline int integrity_inode_alloc(struct inode *inode)
+{
+ return 0;
+}
+
+static inline void integrity_inode_free(struct inode *inode)
+{
+ return;
+}
+
+static inline int integrity_inode_permission(struct file *file,
+ struct path *path, int mask);
+{
+ return 0;
+}
+
+static inline int integrity_file_permission(struct file *file, int mask)
+{
+ return 0;
+}
+
+static inline void integrity_file_free(struct file *file)
+{
+ return;
+}
+
+static inline int integrity_file_mmap(struct file *file, unsigned long prot)
+{
+ return 0;
+}
+#endif
+#endif
Index: security-testing-2.6/mm/mmap.c
===================================================================
--- security-testing-2.6.orig/mm/mmap.c
+++ security-testing-2.6/mm/mmap.c
@@ -20,6 +20,7 @@
#include <linux/fs.h>
#include <linux/personality.h>
#include <linux/security.h>
+#include <linux/integrity.h>
#include <linux/hugetlb.h>
#include <linux/profile.h>
#include <linux/module.h>
@@ -1046,6 +1047,9 @@ unsigned long do_mmap_pgoff(struct file
error = security_file_mmap(file, reqprot, prot, flags, addr, 0);
if (error)
return error;
+ error = integrity_file_mmap(file, prot);
+ if (error)
+ return error;

return mmap_region(file, addr, len, flags, vm_flags, pgoff,
accountable);
Index: security-testing-2.6/security/Kconfig
===================================================================
--- security-testing-2.6.orig/security/Kconfig
+++ security-testing-2.6/security/Kconfig
@@ -4,6 +4,8 @@

menu "Security options"

+source security/integrity/Kconfig
+
config KEYS
bool "Enable access key retention support"
help
Index: security-testing-2.6/security/Makefile
===================================================================
--- security-testing-2.6.orig/security/Makefile
+++ security-testing-2.6/security/Makefile
@@ -16,3 +16,7 @@ obj-$(CONFIG_SECURITY_SELINUX) += selin
obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o
obj-$(CONFIG_SECURITY_ROOTPLUG) += root_plug.o
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
+
+# Object integrity file lists
+subdir-$(CONFIG_INTEGRITY) += integrity
+obj-$(CONFIG_INTEGRITY) += integrity/built-in.o
Index: security-testing-2.6/security/integrity/Kconfig
===================================================================
--- /dev/null
+++ security-testing-2.6/security/integrity/Kconfig
@@ -0,0 +1,24 @@
+#
+# Integrity configuration
+#
+
+menu "Integrity options"
+
+config INTEGRITY
+ bool "Enable different integrity models"
+ help
+ This allows you to choose different integrity modules to be
+ configured into your kernel.
+
+ If you are unsure how to answer this question, answer N.
+
+config INTEGRITY_AUDIT
+ bool "Integrity audit boot parameter"
+ depends on INTEGRITY
+ default y
+ help
+ This option adds a kernel parameter 'integrity_audit', which
+ allows integrity auditing to be disabled at boot. If this
+ option is selected, integrity auditing can be disabled with
+ 'integrity_audit=0' on the kernel command line.
+endmenu
Index: security-testing-2.6/security/integrity/Makefile
===================================================================
--- /dev/null
+++ security-testing-2.6/security/integrity/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for the kernel integrity code
+#
+
+# Object file lists
+obj-$(CONFIG_INTEGRITY) += integrity.o integrity_audit.o
Index: security-testing-2.6/security/integrity/integrity.c
===================================================================
--- /dev/null
+++ security-testing-2.6/security/integrity/integrity.c
@@ -0,0 +1,306 @@
+/*
+ * Copyright (C) 2006,2007,2008 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.
+ *
+ * File: integrity.c
+ * register integrity subsystem
+ * register integrity template
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/integrity.h>
+
+const struct integrity_operations *integrity_ops;
+EXPORT_SYMBOL(integrity_ops);
+
+#define TEMPLATE_NAME_LEN_MAX 12
+struct template_list_entry {
+ struct list_head template;
+ char template_name[TEMPLATE_NAME_LEN_MAX + 1];
+ const struct template_operations *template_ops;
+};
+static LIST_HEAD(integrity_templates);
+static DEFINE_MUTEX(integrity_templates_mutex);
+
+/**
+ * register_integrity - registers an integrity framework with the kernel
+ * @ops: a pointer to the struct security_options that is to be registered
+ *
+ * Perhaps in the future integrity module stacking will be necessary, but
+ * for the time being, this function permits only one integrity module to
+ * register itself with the kernel integrity subsystem.
+ *
+ * If another integrity module is already registered, an error code is
+ * returned. On success 0 is returned.
+ */
+int register_integrity(const struct integrity_operations *ops)
+{
+ if (integrity_ops != NULL)
+ return -EAGAIN;
+ integrity_ops = ops;
+ return 0;
+}
+
+EXPORT_SYMBOL_GPL(register_integrity);
+
+/**
+ * unregister_integrity - unregisters an integrity framework from the kernel
+ * @ops: a pointer to the struct security_options that is to be registered
+ *
+ * Returns 0 on success, -EINVAL on failure.
+ */
+int unregister_integrity(const struct integrity_operations *ops)
+{
+ if (ops != integrity_ops)
+ return -EINVAL;
+
+ integrity_ops = NULL;
+ return 0;
+}
+
+EXPORT_SYMBOL_GPL(unregister_integrity);
+
+/**
+ * integrity_register_template - registers an integrity template with the kernel
+ * @template_name: a pointer to a string containing the template name.
+ * @template_ops: a pointer to the template functions
+ *
+ * Register a set of functions to collect, appraise, store, and display
+ * a template measurement, and a means to decide whether to do them.
+ * Unlike integrity modules, any number of templates may be registered.
+ *
+ * Returns 0 on success, an error code on failure.
+ */
+int integrity_register_template(const char *template_name,
+ const struct template_operations *template_ops)
+{
+ int template_len;
+ struct template_list_entry *entry;
+
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+ INIT_LIST_HEAD(&entry->template);
+
+ template_len = strlen(template_name);
+ if (template_len > TEMPLATE_NAME_LEN_MAX)
+ return -EINVAL;
+ strcpy(entry->template_name, template_name);
+ entry->template_ops = template_ops;
+
+ mutex_lock(&integrity_templates_mutex);
+ list_add_rcu(&entry->template, &integrity_templates);
+ mutex_unlock(&integrity_templates_mutex);
+ synchronize_rcu();
+
+ return 0;
+}
+
+EXPORT_SYMBOL_GPL(integrity_register_template);
+
+/**
+ * integrity_unregister_template: unregister a template
+ * @template_name: a pointer to a string containing the template name.
+ *
+ * Returns 0 on success, -EINVAL on failure.
+ */
+int integrity_unregister_template(const char *template_name)
+{
+ struct template_list_entry *entry;
+
+ mutex_lock(&integrity_templates_mutex);
+ list_for_each_entry(entry, &integrity_templates, template) {
+ if (strncmp(entry->template_name, template_name,
+ strlen(entry->template_name)) == 0) {
+ list_del_rcu(&entry->template);
+ mutex_unlock(&integrity_templates_mutex);
+ synchronize_rcu();
+ kfree(entry);
+ return 0;
+ }
+ }
+ mutex_unlock(&integrity_templates_mutex);
+ return -EINVAL;
+}
+
+EXPORT_SYMBOL_GPL(integrity_unregister_template);
+
+/**
+ * integrity_find_template - search the integrity_templates list
+ * @template_name: a pointer to a string containing the template name.
+ * @template_ops: a pointer to the template functions
+ *
+ * Called with an rcu_read_lock
+ * Returns 0 on success, -EINVAL on failure.
+ */
+int integrity_find_template(const char *template_name,
+ const struct template_operations **template_ops)
+{
+ struct template_list_entry *entry;
+
+ list_for_each_entry_rcu(entry, &integrity_templates, template) {
+ if (strncmp(entry->template_name, template_name,
+ strlen(entry->template_name)) == 0) {
+ *template_ops = entry->template_ops;
+ return 0;
+ }
+ }
+ return -EINVAL;
+}
+
+EXPORT_SYMBOL_GPL(integrity_find_template);
+
+/* Start of the integrity API calls */
+
+/**
+ * integrity_collect_measurement - collect template specific measurement
+ * @template_name: a pointer to a string containing the template name.
+ * @data: pointer to template specific data
+ *
+ * Returns 0 on success, an error code on failure.
+ */
+int integrity_collect_measurement(const char *template_name, void *data)
+{
+ const struct template_operations *template_ops;
+ int rc;
+
+ rcu_read_lock();
+ rc = integrity_find_template(template_name, &template_ops);
+ if (rc == 0)
+ rc = template_ops->collect_measurement(data);
+ rcu_read_unlock();
+ return rc;
+}
+
+EXPORT_SYMBOL_GPL(integrity_collect_measurement);
+
+/**
+ * integrity_appraise_measurement - appraise template specific measurement
+ * @template_name: a pointer to a string containing the template name.
+ * @data: pointer to template specific data
+ *
+ * Returns 0 on success, an error code on failure
+ */
+int integrity_appraise_measurement(const char *template_name, void *data)
+{
+ const struct template_operations *template_ops;
+ int rc;
+
+ rcu_read_lock();
+ rc = integrity_find_template(template_name, &template_ops);
+ if (rc == 0)
+ rc = template_ops->appraise_measurement(data);
+ rcu_read_unlock();
+ return rc;
+}
+
+EXPORT_SYMBOL_GPL(integrity_appraise_measurement);
+
+/**
+ * integrity_store_measurement - store template specific measurement
+ * @template_name: a pointer to a string containing the template name.
+ * @data: pointer to template specific data
+ *
+ * Store template specific integrity measurement.
+ */
+int integrity_store_measurement(const char *template_name, void *data)
+{
+ const struct template_operations *template_ops;
+ int rc;
+
+ rcu_read_lock();
+ rc = integrity_find_template(template_name, &template_ops);
+ if (rc == 0)
+ template_ops->store_measurement(data);
+ rcu_read_unlock();
+ return rc;
+}
+
+EXPORT_SYMBOL_GPL(integrity_store_measurement);
+
+/**
+ * integrity_must_measure - measure decision based on template policy
+ * @template_name: a pointer to a string containing the template name.
+ * @data: pointer to template specific data
+ *
+ * Returns 0 on success, an error code on failure.
+ */
+int integrity_must_measure(const char *template_name, void *data)
+{
+ const struct template_operations *template_ops;
+ int rc;
+
+ rcu_read_lock();
+ rc = integrity_find_template(template_name, &template_ops);
+ if (rc == 0)
+ rc = template_ops->must_measure(data);
+ rcu_read_unlock();
+ return rc;
+}
+
+EXPORT_SYMBOL_GPL(integrity_must_measure);
+
+/* Start of the integrity Hooks */
+
+/* Hook used to measure executable file integrity. */
+int integrity_bprm_check(struct linux_binprm *bprm)
+{
+ int rc = 0;
+
+ if (integrity_ops && integrity_ops->bprm_check_integrity)
+ rc = integrity_ops->bprm_check_integrity(bprm);
+ return rc;
+}
+
+/* Allocate, attach and initialize an inode's i_integrity. */
+int integrity_inode_alloc(struct inode *inode)
+{
+ int rc = 0;
+
+ if (integrity_ops && integrity_ops->inode_alloc_integrity)
+ rc = integrity_ops->inode_alloc_integrity(inode);
+ return rc;
+}
+
+/* Hook used to free an inode's i_integrity structure. */
+void integrity_inode_free(struct inode *inode)
+{
+ if (integrity_ops && integrity_ops->inode_free_integrity)
+ integrity_ops->inode_free_integrity(inode);
+}
+
+/* Hook used to measure a file's integrity. */
+int integrity_inode_permission(struct file *file, struct path *path, int mask)
+{
+ int rc = 0;
+
+ if (integrity_ops && integrity_ops->inode_permission)
+ rc = integrity_ops->inode_permission(file, path, mask);
+ return rc;
+}
+
+/* Hook used to update i_integrity data and integrity xattr values
+ * as necessary.
+ */
+void integrity_file_free(struct file *file)
+{
+ if (integrity_ops && integrity_ops->file_free_integrity)
+ integrity_ops->file_free_integrity(file);
+}
+
+/* Hook used to measure integrity of an mmapped file */
+int integrity_file_mmap(struct file *file, unsigned long prot)
+{
+ int rc = 0;
+
+ if (integrity_ops && integrity_ops->file_mmap)
+ rc = integrity_ops->file_mmap(file, prot);
+ return rc;
+}
Index: security-testing-2.6/security/integrity/integrity_audit.c
===================================================================
--- /dev/null
+++ security-testing-2.6/security/integrity/integrity_audit.c
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2008 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.
+ *
+ * File: integrity_audit.c
+ * Audit calls for the integrity subsystem
+ */
+
+#include <linux/audit.h>
+#include <linux/fs.h>
+#include <linux/integrity.h>
+
+static int integrity_audit = 1;
+
+#ifdef CONFIG_INTEGRITY_AUDIT
+static int __init integrity_audit_setup(char *str)
+{
+ ulong audit;
+ int rc;
+ char *op;
+
+ rc = strict_strtoul(str, 10, &audit);
+ if (rc < 0 || audit > 1)
+ printk(KERN_INFO "integrity: invalid integrity_audit value\n");
+ else
+ integrity_audit = audit;
+
+ op = integrity_audit ? "integrity_audit_enabled" :
+ "integrity_audit_not_enabled";
+ integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, NULL, op, 0);
+ return 1;
+}
+
+__setup("integrity_audit=", integrity_audit_setup);
+#endif
+
+void integrity_audit_msg(int audit_msgno, struct inode *inode,
+ const unsigned char *fname, char *op,
+ char *cause, int result)
+{
+ struct audit_buffer *ab;
+
+ if (!integrity_audit && result == 1)
+ return;
+
+ ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno);
+ audit_log_format(ab, "integrity: pid=%d uid=%u auid=%u",
+ current->pid, current->uid,
+ audit_get_loginuid(current));
+ audit_log_task_context(ab);
+ switch (audit_msgno) {
+ case AUDIT_INTEGRITY_DATA:
+ case AUDIT_INTEGRITY_METADATA:
+ case AUDIT_INTEGRITY_PCR:
+ audit_log_format(ab, " op=%s cause=%s", op, cause);
+ break;
+ case AUDIT_INTEGRITY_HASH:
+ audit_log_format(ab, " op=%s hash=%s", op, cause);
+ break;
+ case AUDIT_INTEGRITY_STATUS:
+ default:
+ audit_log_format(ab, " op=%s", op);
+ }
+ audit_log_format(ab, " comm=");
+ audit_log_untrustedstring(ab, current->comm);
+ if (fname) {
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fname);
+ }
+ if (inode)
+ audit_log_format(ab, " dev=%s ino=%lu",
+ inode->i_sb->s_id, inode->i_ino);
+ audit_log_format(ab, " res=%d", result);
+ audit_log_end(ab);
+}

--


2008-08-09 18:53:50

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

> + if (integrity_inode_alloc(inode)) {
> + if (inode->i_sb->s_op->destroy_inode)
> + inode->i_sb->s_op->destroy_inode(inode);
> + else
> + kmem_cache_free(inode_cachep, (inode));
> + return NULL;
> + }
> +

Please factor this and the lsm failure case out into a single
out_free_inode goto label.


> int vfs_permission(struct nameidata *nd, int mask)
> {
> - return inode_permission(nd->path.dentry->d_inode, mask);
> + int retval;
> +
> + retval = inode_permission(nd->path.dentry->d_inode, mask);
> + if (retval)
> + return retval;
> + return integrity_inode_permission(NULL, &nd->path,
> + mask & (MAY_READ | MAY_WRITE |
> + MAY_EXEC));
> }
>
> /**
> @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
> */
> int file_permission(struct file *file, int mask)
> {
> - return inode_permission(file->f_path.dentry->d_inode, mask);
> + int retval;
> +
> + retval = inode_permission(file->f_path.dentry->d_inode, mask);
> + if (retval)
> + return retval;
> + return integrity_inode_permission(file, NULL,
> + mask & (MAY_READ | MAY_WRITE |
> + MAY_EXEC));

Please put your hook into inode_permission. Note that in inode
permission and lots of callers there is no path available so don't pass
it. Please pass the full MAY_FOO mask for new interfaces and do
filtering that won't break if new ones are introduced.


> +#ifdef CONFIG_INTEGRITY
> + void *i_integrity;
> +#endif

Sorry, but I don't think we can bloat the inode even further for this.

> +/*
> + * integrity.h

don't bother to mention the filename in the top of file comment.

2008-08-10 13:52:45

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:53:40 PM:

> > + if (integrity_inode_alloc(inode)) {
> > + if (inode->i_sb->s_op->destroy_inode)
> > + inode->i_sb->s_op->destroy_inode(inode);
> > + else
> > + kmem_cache_free(inode_cachep, (inode));
> > + return NULL;
> > + }
> > +
>
> Please factor this and the lsm failure case out into a single
> out_free_inode goto label.

ok

> > int vfs_permission(struct nameidata *nd, int mask)
> > {
> > - return inode_permission(nd->path.dentry->d_inode, mask);
> > + int retval;
> > +
> > + retval = inode_permission(nd->path.dentry->d_inode, mask);
> > + if (retval)
> > + return retval;
> > + return integrity_inode_permission(NULL, &nd->path,
> > + mask & (MAY_READ | MAY_WRITE |
> > + MAY_EXEC));
> > }
> >
> > /**
> > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
> > */
> > int file_permission(struct file *file, int mask)
> > {
> > - return inode_permission(file->f_path.dentry->d_inode, mask);
> > + int retval;
> > +
> > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
> > + if (retval)
> > + return retval;
> > + return integrity_inode_permission(file, NULL,
> > + mask & (MAY_READ | MAY_WRITE |
> > + MAY_EXEC));
>
> Please put your hook into inode_permission. Note that in inode
> permission and lots of callers there is no path available so don't pass
> it. Please pass the full MAY_FOO mask for new interfaces and do
> filtering that won't break if new ones are introduced.

We started out with the integrity_inode_permission() hook call in
inode_permission(), but because of the removal of the nameidata
parameter in the last merge, based on discussions
http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
the call to integrity_inode_permission() was moved up to the caller,
where either a file or path are available. Any suggestions?

> > +#ifdef CONFIG_INTEGRITY
> > + void *i_integrity;
> > +#endif
>
> Sorry, but I don't think we can bloat the inode even further for this.

The original version of IMA was LSM based, using i_security. Based
on discussions on the LSM mailing list, it was decided that the LSM hooks
were meant only for access control. During the same time frame, there
was a lot of work done in stacking LSM modules and i_security, but that
approach was dropped. It was suggested that we define a separate set of
hooks for integrity, which this patch set provides. Caching integrity
results is an important aspect. Any suggestions in lieu of defining
i_integrity?

> > +/*
> > + * integrity.h
>
> don't bother to mention the filename in the top of file comment.

ok

Mimi

2008-08-11 17:03:43

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

Quoting Mimi Zohar ([email protected]):
> Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:53:40 PM:
>
> > > + if (integrity_inode_alloc(inode)) {
> > > + if (inode->i_sb->s_op->destroy_inode)
> > > + inode->i_sb->s_op->destroy_inode(inode);
> > > + else
> > > + kmem_cache_free(inode_cachep, (inode));
> > > + return NULL;
> > > + }
> > > +
> >
> > Please factor this and the lsm failure case out into a single
> > out_free_inode goto label.
>
> ok
>
> > > int vfs_permission(struct nameidata *nd, int mask)
> > > {
> > > - return inode_permission(nd->path.dentry->d_inode, mask);
> > > + int retval;
> > > +
> > > + retval = inode_permission(nd->path.dentry->d_inode, mask);
> > > + if (retval)
> > > + return retval;
> > > + return integrity_inode_permission(NULL, &nd->path,
> > > + mask & (MAY_READ | MAY_WRITE |
> > > + MAY_EXEC));
> > > }
> > >
> > > /**
> > > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
> > > */
> > > int file_permission(struct file *file, int mask)
> > > {
> > > - return inode_permission(file->f_path.dentry->d_inode, mask);
> > > + int retval;
> > > +
> > > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
> > > + if (retval)
> > > + return retval;
> > > + return integrity_inode_permission(file, NULL,
> > > + mask & (MAY_READ | MAY_WRITE |
> > > + MAY_EXEC));
> >
> > Please put your hook into inode_permission. Note that in inode
> > permission and lots of callers there is no path available so don't pass
> > it. Please pass the full MAY_FOO mask for new interfaces and do
> > filtering that won't break if new ones are introduced.
>
> We started out with the integrity_inode_permission() hook call in
> inode_permission(), but because of the removal of the nameidata
> parameter in the last merge, based on discussions
> http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
> the call to integrity_inode_permission() was moved up to the caller,
> where either a file or path are available. Any suggestions?

Mimi, can you explain exactly (and concisely) what you are doing with
the pathname?

> > > +#ifdef CONFIG_INTEGRITY
> > > + void *i_integrity;
> > > +#endif
> >
> > Sorry, but I don't think we can bloat the inode even further for this.
>
> The original version of IMA was LSM based, using i_security. Based
> on discussions on the LSM mailing list, it was decided that the LSM hooks
> were meant only for access control. During the same time frame, there
> was a lot of work done in stacking LSM modules and i_security, but that
> approach was dropped. It was suggested that we define a separate set of
> hooks for integrity, which this patch set provides. Caching integrity
> results is an important aspect. Any suggestions in lieu of defining
> i_integrity?

The i_integrity is only bloating the inode if LIM is enabled. Surely
that beats having LIM define its own hash table and locking to track
integrity labels on inodes? Do you have another suggestion?

Or is the concern about having more #ifdefs in the struct inode
definition?

-serge

2008-08-11 19:09:37

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

[email protected] wrote on 08/11/2008 01:02:55 PM:

> Quoting Mimi Zohar ([email protected]):
> > Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:53:40 PM:
> > > > int vfs_permission(struct nameidata *nd, int mask)
> > > > {
> > > > - return inode_permission(nd->path.dentry->d_inode, mask);
> > > > + int retval;
> > > > +
> > > > + retval = inode_permission(nd->path.dentry->d_inode, mask);
> > > > + if (retval)
> > > > + return retval;
> > > > + return integrity_inode_permission(NULL, &nd->path,
> > > > + mask & (MAY_READ | MAY_WRITE |
> > > > + MAY_EXEC));
> > > > }
> > > >
> > > > /**
> > > > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
> > > > */
> > > > int file_permission(struct file *file, int mask)
> > > > {
> > > > - return inode_permission(file->f_path.dentry->d_inode, mask);
> > > > + int retval;
> > > > +
> > > > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
> > > > + if (retval)
> > > > + return retval;
> > > > + return integrity_inode_permission(file, NULL,
> > > > + mask & (MAY_READ | MAY_WRITE |
> > > > + MAY_EXEC));
> > >
> > > Please put your hook into inode_permission. Note that in inode
> > > permission and lots of callers there is no path available so don't
pass
> > > it. Please pass the full MAY_FOO mask for new interfaces and do
> > > filtering that won't break if new ones are introduced.
> >
> > We started out with the integrity_inode_permission() hook call in
> > inode_permission(), but because of the removal of the nameidata
> > parameter in the last merge, based on discussions
> > http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
> > the call to integrity_inode_permission() was moved up to the caller,
> > where either a file or path are available. Any suggestions?
>
> Mimi, can you explain exactly (and concisely) what you are doing with
> the pathname?

IMA maintains a list of hash values of system sensitive files loaded
into the run-time of the system and extends a PCR with the hash value.
In order to calculate this hash value, IMA requires access to either
the file or the path, which currently is not accessible in
inode_permission().

Mimi

2008-08-11 19:57:19

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

Quoting Mimi Zohar ([email protected]):
> [email protected] wrote on 08/11/2008 01:02:55 PM:
>
> > Quoting Mimi Zohar ([email protected]):
> > > Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:53:40 PM:
> > > > > int vfs_permission(struct nameidata *nd, int mask)
> > > > > {
> > > > > - return inode_permission(nd->path.dentry->d_inode, mask);
> > > > > + int retval;
> > > > > +
> > > > > + retval = inode_permission(nd->path.dentry->d_inode, mask);
> > > > > + if (retval)
> > > > > + return retval;
> > > > > + return integrity_inode_permission(NULL, &nd->path,
> > > > > + mask & (MAY_READ | MAY_WRITE |
> > > > > + MAY_EXEC));
> > > > > }
> > > > >
> > > > > /**
> > > > > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
> > > > > */
> > > > > int file_permission(struct file *file, int mask)
> > > > > {
> > > > > - return inode_permission(file->f_path.dentry->d_inode, mask);
> > > > > + int retval;
> > > > > +
> > > > > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
> > > > > + if (retval)
> > > > > + return retval;
> > > > > + return integrity_inode_permission(file, NULL,
> > > > > + mask & (MAY_READ | MAY_WRITE |
> > > > > + MAY_EXEC));
> > > >
> > > > Please put your hook into inode_permission. Note that in inode
> > > > permission and lots of callers there is no path available so don't
> pass
> > > > it. Please pass the full MAY_FOO mask for new interfaces and do
> > > > filtering that won't break if new ones are introduced.
> > >
> > > We started out with the integrity_inode_permission() hook call in
> > > inode_permission(), but because of the removal of the nameidata
> > > parameter in the last merge, based on discussions
> > > http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
> > > the call to integrity_inode_permission() was moved up to the caller,
> > > where either a file or path are available. Any suggestions?
> >
> > Mimi, can you explain exactly (and concisely) what you are doing with
> > the pathname?
>
> IMA maintains a list of hash values of system sensitive files loaded
> into the run-time of the system and extends a PCR with the hash value.
> In order to calculate this hash value, IMA requires access to either
> the file or the path, which currently is not accessible in
> inode_permission().

So the usual question is, if I've done
ln -s /etc/shadow /tmp/shadow
will IMA do the right thing if I'm opening /tmp/shadow? Or will it only
catch any writes I've done the next time someone (i.e. passwd) opens
/etc/shadow?

thanks,
-serge

2008-08-12 08:41:27

by Peter Dolding

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Tue, Aug 12, 2008 at 5:56 AM, Serge E. Hallyn <[email protected]> wrote:
> Quoting Mimi Zohar ([email protected]):
>> [email protected] wrote on 08/11/2008 01:02:55 PM:
>>
>> > Quoting Mimi Zohar ([email protected]):
>> > > Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:53:40 PM:
>> > > > > int vfs_permission(struct nameidata *nd, int mask)
>> > > > > {
>> > > > > - return inode_permission(nd->path.dentry->d_inode, mask);
>> > > > > + int retval;
>> > > > > +
>> > > > > + retval = inode_permission(nd->path.dentry->d_inode, mask);
>> > > > > + if (retval)
>> > > > > + return retval;
>> > > > > + return integrity_inode_permission(NULL, &nd->path,
>> > > > > + mask & (MAY_READ | MAY_WRITE |
>> > > > > + MAY_EXEC));
>> > > > > }
>> > > > >
>> > > > > /**
>> > > > > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
>> > > > > */
>> > > > > int file_permission(struct file *file, int mask)
>> > > > > {
>> > > > > - return inode_permission(file->f_path.dentry->d_inode, mask);
>> > > > > + int retval;
>> > > > > +
>> > > > > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
>> > > > > + if (retval)
>> > > > > + return retval;
>> > > > > + return integrity_inode_permission(file, NULL,
>> > > > > + mask & (MAY_READ | MAY_WRITE |
>> > > > > + MAY_EXEC));
>> > > >
>> > > > Please put your hook into inode_permission. Note that in inode
>> > > > permission and lots of callers there is no path available so don't
>> pass
>> > > > it. Please pass the full MAY_FOO mask for new interfaces and do
>> > > > filtering that won't break if new ones are introduced.
>> > >
>> > > We started out with the integrity_inode_permission() hook call in
>> > > inode_permission(), but because of the removal of the nameidata
>> > > parameter in the last merge, based on discussions
>> > > http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
>> > > the call to integrity_inode_permission() was moved up to the caller,
>> > > where either a file or path are available. Any suggestions?
>> >
>> > Mimi, can you explain exactly (and concisely) what you are doing with
>> > the pathname?
>>
>> IMA maintains a list of hash values of system sensitive files loaded
>> into the run-time of the system and extends a PCR with the hash value.
>> In order to calculate this hash value, IMA requires access to either
>> the file or the path, which currently is not accessible in
>> inode_permission().
>
> So the usual question is, if I've done
> ln -s /etc/shadow /tmp/shadow
> will IMA do the right thing if I'm opening /tmp/shadow? Or will it only
> catch any writes I've done the next time someone (i.e. passwd) opens
> /etc/shadow?
>
> thanks,
> -serge

We really do need to get credentials patch in to common store all this
permission/secuirty data.. With a section for integrity related
entries.

Anti-Virus Passes and fails, signed running programs support and so on.

Lot of different things need ways of recording integrity status's.
Users also need to know if a application does not work is it TPM is it
Anti-virus is it lack of signature.

A common way of extracting what the blockage has to be the way
forward. Since then this can be integrated into file managers and the
like. Running like 12 different tools to find what blocked a program
is kinda wasteful.

Sorting out this storage is kinda critical. People could be running
a few different integrity systems side by side.

Peter Dolding

2008-08-12 19:25:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Sun, Aug 10, 2008 at 09:52:13AM -0400, Mimi Zohar wrote:
> We started out with the integrity_inode_permission() hook call in
> inode_permission(), but because of the removal of the nameidata
> parameter in the last merge, based on discussions
> http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
> the call to integrity_inode_permission() was moved up to the caller,
> where either a file or path are available. Any suggestions?

vfs_permission and file_permission are just small wrappers around
inode_permission. In hindsight they actualyl were a wrong idea and
will probably go away in the not so distant future. Note that there
are various callers of inode_permission that don't have a vfsmount
anywhere near.

2008-08-12 19:27:51

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Mon, Aug 11, 2008 at 12:02:55PM -0500, Serge E. Hallyn wrote:
> > > Sorry, but I don't think we can bloat the inode even further for this.
> >
> > The original version of IMA was LSM based, using i_security. Based
> > on discussions on the LSM mailing list, it was decided that the LSM hooks
> > were meant only for access control. During the same time frame, there
> > was a lot of work done in stacking LSM modules and i_security, but that
> > approach was dropped. It was suggested that we define a separate set of
> > hooks for integrity, which this patch set provides. Caching integrity
> > results is an important aspect. Any suggestions in lieu of defining
> > i_integrity?
>
> The i_integrity is only bloating the inode if LIM is enabled. Surely
> that beats having LIM define its own hash table and locking to track
> integrity labels on inodes? Do you have another suggestion?
>
> Or is the concern about having more #ifdefs in the struct inode
> definition?

No, the concern is over bloating the inode for a rather academic fringe
feature. As this comes from IBM I'm pretty sure someone will pressure
the big distro to turn it on. And inode growth is a concern for
fileserving or other inode heavy workload. Mimi mentioned this is just
a cache of information, so consider using something like XFS's mru cache
which is used for something similar where the xfs_inode was kept small
despite a very niche feature needing a cache attached to the inode:

fs/xfs/xfs_mru_cache.c

2008-08-12 19:29:57

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Tue, Aug 12, 2008 at 06:41:16PM +1000, Peter Dolding wrote:
> We really do need to get credentials patch in to common store all this
> permission/secuirty data.. With a section for integrity related
> entries.
>
> Anti-Virus Passes and fails, signed running programs support and so on.
>
> Lot of different things need ways of recording integrity status's.
> Users also need to know if a application does not work is it TPM is it
> Anti-virus is it lack of signature.

Peter, please read up what the credentials patches do, or how struct
cred/ucred is used in SVR4 and BSD for the last 20 years. It is useful,
but it's not going to help with any of the strange thigns the AV or
Integrity people are doing.

2008-08-12 21:19:30

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

Quoting Christoph Hellwig ([email protected]):
> On Mon, Aug 11, 2008 at 12:02:55PM -0500, Serge E. Hallyn wrote:
> > > > Sorry, but I don't think we can bloat the inode even further for this.
> > >
> > > The original version of IMA was LSM based, using i_security. Based
> > > on discussions on the LSM mailing list, it was decided that the LSM hooks
> > > were meant only for access control. During the same time frame, there
> > > was a lot of work done in stacking LSM modules and i_security, but that
> > > approach was dropped. It was suggested that we define a separate set of
> > > hooks for integrity, which this patch set provides. Caching integrity
> > > results is an important aspect. Any suggestions in lieu of defining
> > > i_integrity?
> >
> > The i_integrity is only bloating the inode if LIM is enabled. Surely
> > that beats having LIM define its own hash table and locking to track
> > integrity labels on inodes? Do you have another suggestion?
> >
> > Or is the concern about having more #ifdefs in the struct inode
> > definition?
>
> No, the concern is over bloating the inode for a rather academic fringe
> feature. As this comes from IBM I'm pretty sure someone will pressure
> the big distro to turn it on.

By default?? I should hope not...

Note that these are all not loadable modules. So presumably either it's
in the kernel and enforcing, or it's not there.

> And inode growth is a concern for
> fileserving or other inode heavy workload. Mimi mentioned this is just
> a cache of information, so consider using something like XFS's mru cache
> which is used for something similar where the xfs_inode was kept small
> despite a very niche feature needing a cache attached to the inode:
>
> fs/xfs/xfs_mru_cache.c

ok, so basically as I said above

> > ... having LIM define its own hash table and locking to track
> > integrity labels on inodes?

:)

But then that is in fact the better way to go if there can be a lot
of inodes with i_integrity=NULL. It looks like IMA always allocates
something, but if I understand the idea behind templates correctly,
that isn't necessarily always the case.

thanks,
-serge

2008-08-13 10:45:21

by Peter Dolding

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Wed, Aug 13, 2008 at 5:29 AM, Christoph Hellwig <[email protected]> wrote:
> On Tue, Aug 12, 2008 at 06:41:16PM +1000, Peter Dolding wrote:
>> We really do need to get credentials patch in to common store all this
>> permission/secuirty data.. With a section for integrity related
>> entries.
>>
>> Anti-Virus Passes and fails, signed running programs support and so on.
>>
>> Lot of different things need ways of recording integrity status's.
>> Users also need to know if a application does not work is it TPM is it
>> Anti-virus is it lack of signature.
>
> Peter, please read up what the credentials patches do, or how struct
> cred/ucred is used in SVR4 and BSD for the last 20 years. It is useful,
> but it's not going to help with any of the strange thigns the AV or
> Integrity people are doing.
>
The Issue I have. By your answer you have not. Credentials patch
for Linux allows more than the BSD one does. Linux one is a complete
permission storage replacement.

http://linux.derkeiler.com/Mailing-Lists/Kernel/2008-08/msg02682.html

Note the file credentials one. That is reused by FS Cache and it
creates fake inodes. So worst case event LIM blocks a valid file
because its coming from cache.

"vfs_permission and file_permission are just small wrappers around
inode_permission." No longer both go to inote_permission after the
credentials patch is in. file_permission instead goes to credentials
struct connected to the inode. Most calls to inode_permission end up
wrapped to the credentials struct.

Basically by the way Linux Credentials patch is being done.
inode_permission could completely cease to exist. Completely
replaced by the credentials structure.

Each filesystem having its own cache is one reason why Linux
Credentials Patch is being brought into live. So a single cache can
store all the need information of the OS and for the file system.
Even better operate on filesystems lacking all the need permission
structs using a userspace program to fill in some of the blanks.

LSM's in Credentials can there own protected data sets since all
alterations to Credentials by the user space deamon have to go past
LSM for approval or rejection. Linux Credentials only need a extra
protected zone added to cover you LIM needs and AV needs to store
data.

In simple terms permissions stored in inodes is basically deprecated
by Linux's Credentials patch.

Sorting out the Credentials patch is kinda key. Nothing you AV or
Integrity people is strange to the Linux Credentials patch. Without
embracing requires more processing when pulling data from a common
cache that has already been AV or Integrity scanned and maintained in
that state. Now its really designing the struct that should exist in
Credentials.

Peter Dolding

2008-08-13 14:18:41

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

Peter Dolding <[email protected]> wrote:

> Credentials patch for Linux allows more than the BSD one does. Linux one is
> a complete permission storage replacement.

No, it isn't. It replaces the permission storage facility of a process but
does not change the way in which an inode caches its credentials in RAM.

> http://linux.derkeiler.com/Mailing-Lists/Kernel/2008-08/msg02682.html
>
> Note the file credentials one.

I said "Open file credentials". I was referring to struct file which
designates an open file handle, not the file/inode itself. What this means is
that the open file handle takes a reference to the subjective security context
of the process that opened it, at the time it opened it, so that accesses to
that file are (with exceptions) done with those credentials, no matter who is
actually using the handle.

For disk filesystems, for the most part, this makes no difference: an open file
is usable by anyone who has the handle. However, for network filesystems,
where each request is marked with the details of who the request should be
accounted to and the server decides, it does matter.

> That is reused by FS Cache and it creates fake inodes.

FS-Cache+CacheFiles doesn't create fake inodes at all. It creates real inodes
on disk in the cache. The reason for these patches is that CacheFiles needs to
use its own subjective security context when creating or accessing these
inodes, rather than the subjective context of whoever invoked AFS or NFS, and
thus caused CacheFiles to touch the cache.

> "vfs_permission and file_permission are just small wrappers around
> inode_permission." No longer both go to inote_permission after the
> credentials patch is in. file_permission instead goes to credentials
> struct connected to the inode. Most calls to inode_permission end up
> wrapped to the credentials struct.

I don't understand what you're talking about. Looking in fs/namei.c, I see:

int vfs_permission(struct nameidata *nd, int mask)
{
return inode_permission(nd->path.dentry->d_inode, mask);
}

And:

int file_permission(struct file *file, int mask)
{
return inode_permission(file->f_path.dentry->d_inode, mask);
}

That looks like they still both go to inode_permission().

> Basically by the way Linux Credentials patch is being done.
> inode_permission could completely cease to exist. Completely
> replaced by the credentials structure.

Erm... Why? inode_permission() is a function, the creds struct is a data
type.

> Each filesystem having its own cache is one reason why Linux Credentials
> Patch is being brought into live.

Not as far as I know. The credentials belonging to the filesystem and the
inodes in the filesystem belong to the filesystem and are not held in cred
structs. Using cred structs for inodes would waste memory as a task's
credentials have a lot of bits an inode's don't.

> So a single cache can store all the need information of the OS and for the
> file system. Even better operate on filesystems lacking all the need
> permission structs using a userspace program to fill in some of the blanks.
>
> LSM's in Credentials can there own protected data sets since all
> alterations to Credentials by the user space deamon have to go past
> LSM for approval or rejection. Linux Credentials only need a extra
> protected zone added to cover you LIM needs and AV needs to store
> data.

Are you suggesting the offloading of security data caching to userspace?

> In simple terms permissions stored in inodes is basically deprecated
> by Linux's Credentials patch.

In simple terms, no they aren't.

> Sorting out the Credentials patch is kinda key. Nothing you AV or
> Integrity people is strange to the Linux Credentials patch. Without
> embracing requires more processing when pulling data from a common
> cache that has already been AV or Integrity scanned and maintained in
> that state. Now its really designing the struct that should exist in
> Credentials.

>From my quick glance over the integrity patches, I'd say that the cred struct
is entirely the wrong place to store this data. It's nothing to do with a
task's subjective state, except, perhaps, in setting it up. It appears to be
per-inode state.

David

2008-08-13 17:04:16

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Tue, 2008-08-12 at 16:19 -0500, Serge E. Hallyn wrote:
> Quoting Christoph Hellwig ([email protected]):
> > On Mon, Aug 11, 2008 at 12:02:55PM -0500, Serge E. Hallyn wrote:
> > > > > Sorry, but I don't think we can bloat the inode even further for this.
> > > >
> > > > The original version of IMA was LSM based, using i_security. Based
> > > > on discussions on the LSM mailing list, it was decided that the LSM hooks
> > > > were meant only for access control. During the same time frame, there
> > > > was a lot of work done in stacking LSM modules and i_security, but that
> > > > approach was dropped. It was suggested that we define a separate set of
> > > > hooks for integrity, which this patch set provides. Caching integrity
> > > > results is an important aspect. Any suggestions in lieu of defining
> > > > i_integrity?
> > >
> > > The i_integrity is only bloating the inode if LIM is enabled. Surely
> > > that beats having LIM define its own hash table and locking to track
> > > integrity labels on inodes? Do you have another suggestion?
> > >
> > > Or is the concern about having more #ifdefs in the struct inode
> > > definition?
> >
> > No, the concern is over bloating the inode for a rather academic fringe
> > feature. As this comes from IBM I'm pretty sure someone will pressure
> > the big distro to turn it on.
>
> By default?? I should hope not...
>
> Note that these are all not loadable modules. So presumably either it's
> in the kernel and enforcing, or it's not there.
>
> > And inode growth is a concern for
> > fileserving or other inode heavy workload. Mimi mentioned this is just
> > a cache of information, so consider using something like XFS's mru cache
> > which is used for something similar where the xfs_inode was kept small
> > despite a very niche feature needing a cache attached to the inode:
> >
> > fs/xfs/xfs_mru_cache.c
>
> ok, so basically as I said above
>
> > > ... having LIM define its own hash table and locking to track
> > > integrity labels on inodes?
>
> :)
>
> But then that is in fact the better way to go if there can be a lot
> of inodes with i_integrity=NULL. It looks like IMA always allocates
> something, but if I understand the idea behind templates correctly,
> that isn't necessarily always the case.
>
> thanks,
> -serge

IMA has a two stage initialization, one at security_initcall() and
another at late_initcall(), when the tpm is available, to make sure
that all inode's i_integrity are allocated.

Multiple templates can register themselves with LIM, but only one
integrity provider, such as IMA, can register itself at a time. So
hypothetically, other integrity providers could be implemented
without a need for i_integrity.

Mimi

2008-08-13 17:04:49

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Mon, 2008-08-11 at 14:56 -0500, Serge E. Hallyn wrote:
> Quoting Mimi Zohar ([email protected]):
> > [email protected] wrote on 08/11/2008 01:02:55 PM:
> >
> > > Quoting Mimi Zohar ([email protected]):
> > > > Christoph Hellwig <[email protected]> wrote on 08/09/2008 02:53:40 PM:
> > > > > > int vfs_permission(struct nameidata *nd, int mask)
> > > > > > {
> > > > > > - return inode_permission(nd->path.dentry->d_inode, mask);
> > > > > > + int retval;
> > > > > > +
> > > > > > + retval = inode_permission(nd->path.dentry->d_inode, mask);
> > > > > > + if (retval)
> > > > > > + return retval;
> > > > > > + return integrity_inode_permission(NULL, &nd->path,
> > > > > > + mask & (MAY_READ | MAY_WRITE |
> > > > > > + MAY_EXEC));
> > > > > > }
> > > > > >
> > > > > > /**
> > > > > > @@ -306,7 +314,14 @@ int vfs_permission(struct nameidata *nd,
> > > > > > */
> > > > > > int file_permission(struct file *file, int mask)
> > > > > > {
> > > > > > - return inode_permission(file->f_path.dentry->d_inode, mask);
> > > > > > + int retval;
> > > > > > +
> > > > > > + retval = inode_permission(file->f_path.dentry->d_inode, mask);
> > > > > > + if (retval)
> > > > > > + return retval;
> > > > > > + return integrity_inode_permission(file, NULL,
> > > > > > + mask & (MAY_READ | MAY_WRITE |
> > > > > > + MAY_EXEC));
> > > > >
> > > > > Please put your hook into inode_permission. Note that in inode
> > > > > permission and lots of callers there is no path available so don't
> > pass
> > > > > it. Please pass the full MAY_FOO mask for new interfaces and do
> > > > > filtering that won't break if new ones are introduced.
> > > >
> > > > We started out with the integrity_inode_permission() hook call in
> > > > inode_permission(), but because of the removal of the nameidata
> > > > parameter in the last merge, based on discussions
> > > > http://marc.info/?l=linux-security-module&m=121797845308246&w=2,
> > > > the call to integrity_inode_permission() was moved up to the caller,
> > > > where either a file or path are available. Any suggestions?
> > >
> > > Mimi, can you explain exactly (and concisely) what you are doing with
> > > the pathname?
> >
> > IMA maintains a list of hash values of system sensitive files loaded
> > into the run-time of the system and extends a PCR with the hash value.
> > In order to calculate this hash value, IMA requires access to either
> > the file or the path, which currently is not accessible in
> > inode_permission().
>
> So the usual question is, if I've done
> ln -s /etc/shadow /tmp/shadow
> will IMA do the right thing if I'm opening /tmp/shadow? Or will it only
> catch any writes I've done the next time someone (i.e. passwd) opens
> /etc/shadow?

It will measure the right thing, as the measurement is based on the dentry
contained within the file/path structures, not names. File names in IMA
are only used as a hint in the measurement hash list.

Mimi

2008-08-13 22:57:55

by Peter Dolding

[permalink] [raw]
Subject: Re: [PATCH 3/4] integrity: Linux Integrity Module(LIM)

On Thu, Aug 14, 2008 at 12:11 AM, David Howells <[email protected]> wrote:
> Peter Dolding <[email protected]> wrote:
>
>> Credentials patch for Linux allows more than the BSD one does. Linux one is
>> a complete permission storage replacement.
>
> No, it isn't. It replaces the permission storage facility of a process but
> does not change the way in which an inode caches its credentials in RAM.
>
>> http://linux.derkeiler.com/Mailing-Lists/Kernel/2008-08/msg02682.html
>>
>> Note the file credentials one.
>
> I said "Open file credentials". I was referring to struct file which
> designates an open file handle, not the file/inode itself. What this means is
> that the open file handle takes a reference to the subjective security context
> of the process that opened it, at the time it opened it, so that accesses to
> that file are (with exceptions) done with those credentials, no matter who is
> actually using the handle.
>
> For disk filesystems, for the most part, this makes no difference: an open file
> is usable by anyone who has the handle. However, for network filesystems,
> where each request is marked with the details of who the request should be
> accounted to and the server decides, it does matter.
>
>> That is reused by FS Cache and it creates fake inodes.
>
> FS-Cache+CacheFiles doesn't create fake inodes at all. It creates real inodes
> on disk in the cache. The reason for these patches is that CacheFiles needs to
> use its own subjective security context when creating or accessing these
> inodes, rather than the subjective context of whoever invoked AFS or NFS, and
> thus caused CacheFiles to touch the cache.
>
Source of Inodes is different. Old style attack aganst AV's has been
to get them to approve something then swap the contents while its
still approved. Simplest is cache attacking. We need to single up
the cache with secuirty around it.

Other cache that has been targeted is the swapfile. Since its another
cache. Building solid intergeity its key to get control of the
caches.

>> "vfs_permission and file_permission are just small wrappers around
>> inode_permission." No longer both go to inote_permission after the
>> credentials patch is in. file_permission instead goes to credentials
>> struct connected to the inode. Most calls to inode_permission end up
>> wrapped to the credentials struct.
>
> I don't understand what you're talking about. Looking in fs/namei.c, I see:
>
> int vfs_permission(struct nameidata *nd, int mask)
> {
> return inode_permission(nd->path.dentry->d_inode, mask);
> }
>
> And:
>
> int file_permission(struct file *file, int mask)
> {
> return inode_permission(file->f_path.dentry->d_inode, mask);
> }
>
> That looks like they still both go to inode_permission().

Hmm I had redirect file_permission in my own working tree. Experiment
in reduced storage in the inode and credentionals systems.

If you look at inode in operation you might as well call the file
permissions on a per file base. All found so far are the pointer to
the same struct if its exactly the same file.

Its a point of duplication of storage. Credentials and inode storing it.

Ie file permissions only stored once.
>> Basically by the way Linux Credentials patch is being done.
>> inode_permission could completely cease to exist. Completely
>> replaced by the credentials structure.
>
> Erm... Why? inode_permission() is a function, the creds struct is a data
> type.
>
>> Each filesystem having its own cache is one reason why Linux Credentials
>> Patch is being brought into live.
>
> Not as far as I know. The credentials belonging to the filesystem and the
> inodes in the filesystem belong to the filesystem and are not held in cred
> structs. Using cred structs for inodes would waste memory as a task's
> credentials have a lot of bits an inode's don't.
>
>> So a single cache can store all the need information of the OS and for the
>> file system. Even better operate on filesystems lacking all the need
>> permission structs using a userspace program to fill in some of the blanks.
>>
>> LSM's in Credentials can there own protected data sets since all
>> alterations to Credentials by the user space deamon have to go past
>> LSM for approval or rejection. Linux Credentials only need a extra
>> protected zone added to cover you LIM needs and AV needs to store
>> data.
>
> Are you suggesting the offloading of security data caching to userspace?

Not for caching mainly. For some operations intergretiy might be
chosen to be done in user space like if file is signed or not. Like
checking just using userspace drivers. Remember you can already
insert userspace drivers in theory for testing of a TPM setup running
from userspace in a small section of the OS could be useful to make
sure everything is right before making active.

Strict interface allowing sections of the security system to operate
in user space and kernel space. For allowing existence of a combined
defence.

AV program may be approved to set X integrity data in credentials.

Signature checking program might set a flag to say that a application
should never run as root or that its a particular class of
application. Triggering the LSM to auto reduce applications that
programs system access to match its settings. For this to be
possible we need secuirty that only the Signature checking program can
set the signature flags. Then the LSM can trust and repond to them.
This is bring ipset's style for firewalls http://ipset.netfilter.org/
to the main secuirty system.

Having both kernel and usermode existing in secuirty has the same
advantage as allowing user mode drivers. The userspace to be
changeable with threats and independent to kernel.

Most creative expands I am looking into.

Giving Linux equal to layor 7 file wall filtering on the filesystem
level. file id magics run on file when before the file is opened by
the application that information stored in credentials. If a program
is no meant to open that file type it can be gets blocked. When
running anti-virus and many filemangers side by side they don't need
to run magics repetitively.

Final one is a new form of file interlink. Bit like streams in NTFS.
So thumbnails and the like of images used by filemanagers are
connected to there related file. So when deleting the file a thumb
nail was created from the thumbnail gets removed. Hopefully allowing
applications to save on down scaling as well.

Yes I am really messing around in the current internal structs. To
hopefully provide a framework to archive the max secuirty creatable.
As well as reducing duplication. So price of doing MAX should be
quite min.
>
>> In simple terms permissions stored in inodes is basically deprecated
>> by Linux's Credentials patch.
>
> In simple terms, no they aren't.

>From the way I am doing it. Storing shared permissions between files
it is deprecated for the inodes stuct. Since I altered the code to
move that into credentials alone. I was thinking too much in my
working tree.
>
>> Sorting out the Credentials patch is kinda key. Nothing you AV or
>> Integrity people is strange to the Linux Credentials patch. Without
>> embracing requires more processing when pulling data from a common
>> cache that has already been AV or Integrity scanned and maintained in
>> that state. Now its really designing the struct that should exist in
>> Credentials.
>
> From my quick glance over the integrity patches, I'd say that the cred struct
> is entirely the wrong place to store this data. It's nothing to do with a
> task's subjective state, except, perhaps, in setting it up. It appears to be
> per-inode state.
>

Its the in operation of the LIM. Same as AV. Complete files will
be scanned. Passed or failed or marked unknown. Currently we have
nice duplication of lots of things. Its been the alteration in my
working tree that has given me major different look at how to do
this. I am trying to get to a location where there is no
duplication of permission storage or caching. Simpler to audit
system access 1 struct know operational and real storage state.

Peter Dolding