[RFC PATCH v2 09/11] ima: load policy using path
From: Dmitry Kasatkin <d.kasat...@samsung.com>
We currently cannot do appraisal or signature vetting of IMA policies
since we currently can only load IMA policies by writing the contents
of the policy directly in, as follows:
cat policy-file > /ima/policy
If we provide the kernel the path to the IMA policy so it can load
the policy itself it'd be able to later appraise or vet the file
signature if it has one. This patch adds support to load the IMA
policy with a given path as follows:
echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
This patch defines kernel_read_file_from_path(), a wrapper for the VFS
common kernel_read_file(), and replaces the integrity_read_file() with
a call to the kernel_read_file_from_path() wrapper.
Changelog v3:
- after re-ordering the patches, replace calling integrity_kernel_read()
to read the file with kernel_read_file_from_path() (Mimi)
Changelog v2:
- Patch description re-written by Luis R. Rodriguez
Signed-off-by: Dmitry Kasatkin <d.kasat...@samsung.com>
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 21
include/linux/fs.h | 1 +
include/linux/ima.h | 1 +
security/integrity/ima/ima_fs.c | 43 +++--
4 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 3524e5f..5731b40 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -903,6 +903,27 @@ out:
return ret;
}
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+ loff_t max_size, int policy_id)
+{
+ struct file *file;
+ int ret;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file)) {
+ ret = PTR_ERR(file);
+ pr_err("Unable to open file: %s (%d)", path, ret);
+ return ret;
+ }
+
+ ret = kernel_read_file(file, buf, size, max_size, policy_id);
+ fput(file);
+ return ret;
+}
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t
len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, );
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9642623..79b1172 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2529,6 +2529,7 @@ extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index eec5e2b..164d918 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -18,6 +18,7 @@ enum ima_policy_id {
INITRAMFS_CHECK,
FIRMWARE_CHECK,
MODULE_CHECK,
+ POLICY_CHECK,
IMA_MAX_READ_CHECK
};
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index f355231..fe8b16b 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include "ima.h"
@@ -258,6 +259,41 @@ static const struct file_operations
ima_ascii_measurements_ops = {
.release = seq_release,
};
+static ssize_t ima_read_policy(char *path)
+{
+ void *data;
+ char *datap;
+ loff_t size;
+ int rc, pathlen = strlen(path);
+
+ char *p;
+
+ /* remove \n */
+ datap = path;
+ strsep(, "\n");
+
+ rc = kernel_read_file_from_path(path, , , 0, POLICY_CHECK);
+ if (rc < 0)
+ return rc;
+
+ datap = data;
+ while (size > 0 && (p = strsep(, "\n"))) {
+ pr_debug("rule: %s\n", p);
+ rc = ima_parse_add_rule(p);
+ if (rc < 0)
+ break;
+ size -= rc;
+ }
+
+ vfree(data);
+ if (rc < 0)
+ return rc;
+ else if (size)
+ return -EINVAL;
+ else
+ return pathlen;
+}
+
static ssize_t ima_write_policy(struct file *file, const char __user *buf,
size_t datalen, loff_t *ppos)
{
@@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file, const
char __user *buf,
result = mutex_lock_interruptible(_write_mutex);
if (result < 0)
goto out_free;
- result = ima_parse_add_rule(data);
- mutex_unlock(_write_mutex);
+ if (data[0] == '/')
+
[RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version
This patch defines kernel_read_file_from_fd(), a wrapper for the VFS
common kernel_read_file(), and replaces the kexec copy_file_from_fd()
calls with the kernel_read_file_from_fd() wrapper.
Two new IMA policy identifiers named KEXEC_CHECK and INITRAMFS_CHECK
are defined for measuring, appraising or auditing the kexec image
and initramfs.
Changelog v1:
- re-order and squash the kexec patches
v3: ima: measure and appraise kexec image and initramfs (squashed)
- rename ima_read_hooks enumeration to ima_policy_id
- use kstat file size type loff_t, not size_t
- add union name "hooks" to fix sparse warning
v2:
- Calculate the file hash from the in memory buffer
(suggested by Dave Young)
- Rename ima_read_and_process_file() to ima_hash_and_process_file()
- replace individual case statements with range:
KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1
v1:
- Instead of ima_read_and_process_file() allocating memory, the caller
allocates and frees the memory.
- Moved the kexec measurement/appraisal call to copy_file_from_fd(). The
same call now measures and appraises both the kexec image and initramfs.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
Documentation/ABI/testing/ima_policy | 2 +-
fs/exec.c | 15
include/linux/fs.h| 1 +
include/linux/ima.h | 2 +
kernel/kexec_file.c | 72 ---
security/integrity/ima/ima.h | 9 -
security/integrity/ima/ima_appraise.c | 7
security/integrity/ima/ima_policy.c | 27 ++---
8 files changed, 64 insertions(+), 71 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy
b/Documentation/ABI/testing/ima_policy
index 0a378a8..e80f767 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -26,7 +26,7 @@ Description:
option: [[appraise_type=]] [permit_directio]
base: func:=
[BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
- [FIRMWARE_CHECK]
+ [FIRMWARE_CHECK] [KEXEC_CHECK] [INITRAMFS_CHECK]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
diff --git a/fs/exec.c b/fs/exec.c
index 211b81c..a5ae51e 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -884,6 +884,21 @@ out:
}
EXPORT_SYMBOL_GPL(kernel_read_file);
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+int policy_id)
+{
+ struct fd f = fdget(fd);
+ int ret = -ENOEXEC;
+
+ if (!f.file)
+ goto out;
+
+ ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+ fdput(f);
+ return ret;
+}
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t
len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, );
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9b1468c..9642623 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index ca76f60..ae91938 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -14,6 +14,8 @@
struct linux_binprm;
enum ima_policy_id {
+ KEXEC_CHECK = 1,
+ INITRAMFS_CHECK,
IMA_MAX_READ_CHECK
};
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b70ada0..f7c3ce4 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -33,65 +34,6 @@ size_t __weak kexec_purgatory_size = 0;
static int kexec_calculate_store_digests(struct kimage *image);
-static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len)
-{
- struct fd f = fdget(fd);
- int ret;
- struct kstat stat;
- loff_t pos;
- ssize_t bytes = 0;
-
- if (!f.file)
- return -EBADF;
-
- ret = vfs_getattr(>f_path, );
- if (ret)
- goto out;
-
- if (stat.size > INT_MAX) {
- ret = -EFBIG;
- goto out;
- }
-
- /* Don't hand 0 to vmalloc, it whines. */
- if (stat.size == 0) {
- ret = -EINVAL;
- goto out;
- }
-
- *buf = vmalloc(stat.size);
- if (!*buf) {
- ret = -ENOMEM;
-
[RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version
This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function. Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.
This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file. For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.
This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 4 +++
include/linux/ima.h | 1 +
include/linux/lsm_hooks.h | 8 +
include/linux/security.h | 3 +-
kernel/module.c | 67 ---
security/integrity/ima/ima.h | 1 -
security/integrity/ima/ima_appraise.c | 7
security/integrity/ima/ima_main.c | 5 ++-
security/integrity/ima/ima_policy.c | 16 -
security/integrity/integrity.h| 12 +++
security/security.c | 12 +--
11 files changed, 47 insertions(+), 89 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index a5ae51e..3524e5f 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -842,6 +842,10 @@ int kernel_read_file(struct file *file, void **buf, loff_t
*size,
if (!S_ISREG(file_inode(file)->i_mode))
return -EINVAL;
+ ret = security_kernel_read_file(file, policy_id);
+ if (ret)
+ return ret;
+
i_size = i_size_read(file_inode(file));
if (max_size > 0 && i_size > max_size)
return -EFBIG;
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0a7f039..eec5e2b 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -17,6 +17,7 @@ enum ima_policy_id {
KEXEC_CHECK = 1,
INITRAMFS_CHECK,
FIRMWARE_CHECK,
+ MODULE_CHECK,
IMA_MAX_READ_CHECK
};
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4e6e2af..9915310 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -561,6 +561,12 @@
* the kernel module to load. If the module is being loaded from a blob,
* this argument will be NULL.
* Return 0 if permission is granted.
+ * @kernel_read_file:
+ * Read a file specified by userspace.
+ * @file contains the file structure pointing to the file being read
+ * by the kernel.
+ * @policy_id contains IMA policy identifier.
+ * Return 0 if permission is granted.
* @kernel_post_read_file:
* Read a file specified by userspace.
* @file contains the file structure pointing to the file being read
@@ -1465,6 +1471,7 @@ union security_list_options {
int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_from_file)(struct file *file);
+ int (*kernel_read_file)(struct file *file, int policy_id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
int policy_id);
int (*task_fix_setuid)(struct cred *new, const struct cred *old,
@@ -1726,6 +1733,7 @@ struct security_hook_heads {
struct list_head kernel_act_as;
struct list_head kernel_create_files_as;
struct list_head kernel_fw_from_file;
+ struct list_head kernel_read_file;
struct list_head kernel_post_read_file;
struct list_head kernel_module_request;
struct list_head kernel_module_from_file;
diff --git a/include/linux/security.h b/include/linux/security.h
index 51f3bc6..6d005b3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -301,6 +301,7 @@ int security_kernel_act_as(struct cred *new, u32 secid);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
int security_kernel_module_request(char *kmod_name);
int security_kernel_module_from_file(struct file *file);
+int security_kernel_read_file(struct file *file, int policy_id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
int policy_id);
int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -857,7 +858,7 @@ static inline int security_kernel_module_request(char
*kmod_name)
return 0;
}
-static inline int security_kernel_module_from_file(struct file *file)
+static inline int security_kernel_read_file(struct file *file, int policy_id)
{
return 0;
}
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..7398d12 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2665,7 +2665,7 @@ static int copy_module_from_user(const void __user *umod,
unsigned long len,
if (info-&g
[RFC PATCH v2 05/11] ima: define a new hook to measure and appraise a file already in memory
This patch defines a new IMA hook ima_hash_and_process_file() for
measuring and appraising files read by the kernel. The caller loads
the file into memory before calling this function, which calculates
the hash followed by the normal IMA policy based processing.
To differentiate between callers of ima_hash_and_process_file() this
patch introducees a policy identifier enumation named ima_policy_id.
Changelog v1:
- To simplify patch review, separate the IMA changes from the kexec
and initramfs changes in "ima: measure and appraise kexec image and
initramfs" patch. This patch contains just the IMA changes. The
kexec and initramfs changes are with the rest of the kexec changes
in "kexec: replace call to copy_file_from_fd() with kernel version".
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 4 +--
include/linux/fs.h| 2 +-
include/linux/ima.h | 14
include/linux/lsm_hooks.h | 4 ++-
include/linux/security.h | 6 ++--
security/integrity/iint.c | 1 +
security/integrity/ima/ima.h | 12 +++
security/integrity/ima/ima_api.c | 6 ++--
security/integrity/ima/ima_appraise.c | 4 +--
security/integrity/ima/ima_main.c | 40 ++-
security/integrity/ima/ima_policy.c | 60 +++
security/integrity/integrity.h| 7 ++--
security/security.c | 6 ++--
13 files changed, 110 insertions(+), 56 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 6d623c2..211b81c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -833,7 +833,7 @@ int kernel_read(struct file *file, loff_t offset,
EXPORT_SYMBOL(kernel_read);
int kernel_read_file(struct file *file, void **buf, loff_t *size,
-loff_t max_size)
+loff_t max_size, int policy_id)
{
loff_t i_size, pos;
ssize_t bytes = 0;
@@ -871,7 +871,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t
*size,
goto out;
}
- ret = security_kernel_post_read_file(file, *buf, i_size);
+ ret = security_kernel_post_read_file(file, *buf, i_size, policy_id);
if (!ret)
*size = pos;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 93ca379..9b1468c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2527,7 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
-extern int kernel_read_file(struct file *, void **, loff_t *, loff_t);
+extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 120ccc5..ca76f60 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -13,6 +13,10 @@
#include
struct linux_binprm;
+enum ima_policy_id {
+ IMA_MAX_READ_CHECK
+};
+
#ifdef CONFIG_IMA
extern int ima_bprm_check(struct linux_binprm *bprm);
extern int ima_file_check(struct file *file, int mask, int opened);
@@ -20,6 +24,9 @@ extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern int ima_module_check(struct file *file);
extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
+extern int ima_hash_and_process_file(struct file *file,
+void *buf, loff_t size,
+enum ima_policy_id policy_id);
#else
static inline int ima_bprm_check(struct linux_binprm *bprm)
@@ -52,6 +59,13 @@ static inline int ima_fw_from_file(struct file *file, char
*buf, size_t size)
return 0;
}
+static inline int ima_hash_and_process_file(struct file *file,
+ void *buf, loff_t size,
+ enum ima_policy_id policy_id)
+{
+ return 0;
+}
+
#endif /* CONFIG_IMA */
#ifdef CONFIG_IMA_APPRAISE
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index f82631c..4e6e2af 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -567,6 +567,7 @@
* by the kernel.
* @buf pointer to buffer containing the file contents.
* @size length of the file contents.
+ * @policy-id IMA policy identifier
* Return 0 if permission is granted.
* @task_fix_setuid:
* Update the module's state after setting one or more of the user
@@ -1464,7 +1465,8 @@ union security_list_options {
int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_fr
[RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version
Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function. A benefit of calling kernel_read_file()
to read the firmware is the firmware is read only once, instead of once
for measuring/appraising the firmware and again for reading the file
contents into memory.
This patch retains the kernel_fw_from_file() hook, which is called from
security_kernel_post_read_file(), but removes the
sercurity_kernel_fw_from_file() function.
Changelog:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
drivers/base/firmware_class.c | 48 ++-
include/linux/ima.h | 7 +
include/linux/security.h | 8 +-
security/integrity/ima/ima.h | 1 -
security/integrity/ima/ima_appraise.c | 8 --
security/integrity/ima/ima_main.c | 18 +
security/integrity/ima/ima_policy.c | 26 +--
security/integrity/integrity.h| 11 +++-
security/security.c | 28 ++--
9 files changed, 54 insertions(+), 101 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8524450..cc175f1 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
@@ -291,40 +292,10 @@ static const char * const fw_path[] = {
module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
MODULE_PARM_DESC(path, "customized firmware image search path with a higher
priority than default path");
-static int fw_read_file_contents(struct file *file, struct firmware_buf
*fw_buf)
-{
- int size;
- char *buf;
- int rc;
-
- if (!S_ISREG(file_inode(file)->i_mode))
- return -EINVAL;
- size = i_size_read(file_inode(file));
- if (size <= 0)
- return -EINVAL;
- buf = vmalloc(size);
- if (!buf)
- return -ENOMEM;
- rc = kernel_read(file, 0, buf, size);
- if (rc != size) {
- if (rc > 0)
- rc = -EIO;
- goto fail;
- }
- rc = security_kernel_fw_from_file(file, buf, size);
- if (rc)
- goto fail;
- fw_buf->data = buf;
- fw_buf->size = size;
- return 0;
-fail:
- vfree(buf);
- return rc;
-}
-
static int fw_get_filesystem_firmware(struct device *device,
struct firmware_buf *buf)
{
+ loff_t size;
int i, len;
int rc = -ENOENT;
char *path;
@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device
*device,
file = filp_open(path, O_RDONLY, 0);
if (IS_ERR(file))
continue;
- rc = fw_read_file_contents(file, buf);
+
+ buf->size = 0;
+ rc = kernel_read_file(file, >data, , INT_MAX,
+ FIRMWARE_CHECK);
fput(file);
if (rc)
dev_warn(device, "firmware, attempted to load %s, but
failed with error %d\n",
path, rc);
- else
+ else {
+ buf->size = (size_t) size;
break;
+ }
}
__putname(path);
@@ -685,8 +661,10 @@ static ssize_t firmware_loading_store(struct device *dev,
dev_err(dev, "%s: map pages failed\n",
__func__);
else
- rc = security_kernel_fw_from_file(NULL,
- fw_buf->data, fw_buf->size);
+ rc = security_kernel_post_read_file(NULL,
+ fw_buf->data,
+ fw_buf->size,
+ FIRMWARE_CHECK);
/*
* Same logic as fw_load_abort, only the DONE bit
diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -16,6 +16,7 @@ struct linux_binprm;
enum ima_policy_id {
KEXEC_CHECK = 1,
INITRAMFS_CHECK,
+ FIRMWARE_CHECK,
IMA_MAX_READ_CHECK
};
@@ -25,7 +26,6 @@ extern int ima_file_check(struct file *file, int mask, int
opened);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern int ima_module_check(struct file *file);
-extern int ima_fw_from_file(struct file *file, char *bu
[RFC PATCH v2 00/11] vfss: support for a common kernel file loader
For a while it was looked down upon to directly read files from Linux. These days there exists a few mechanisms in the kernel that do just this though to load a file into a local buffer. There are minor but important checks differences on each, we should take all the best practices from each of them, generalize them and make all places in the kernel that read a file use it.[1] One difference is the method for opening the file. In some cases we have a file, while in other cases we have a pathname or a file descriptor. Another difference is the security hook calls, or lack of them. In some versions there is a post file read hook, while in others there is a pre file read hook. This patch set is the first attempt at resolving these differences. It does not attempt to merge the different methods of opening a file, but defines a single common kernel file read function with two wrappers. Although this patch set defines two new security hooks for pre and post file read, it does not attempt to merge the existing security hooks. That is left as future work. Changelog v2: - Combined the "ima: measuring/appraising files read by the kernel" patches with this patch set to simplify review. - Split the "ima: measure and appraise kexec image and initramfs" patch to separate IMA from the kexec changes. The latest version of these patches can be found in the next-kernel-read-v2 branch of: git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git [1] Taken from Luis Rodriguez's wiki - http://kernelnewbies.org/KernelProjects/common-kernel-loader Mimi Dmitry Kasatkin (3): ima: separate 'security.ima' reading functionality from collect ima: provide buffer hash calculation function ima: load policy using path Mimi Zohar (8): vfs: define a generic function to read a file from the kernel ima: calculate the hash of a buffer using aynchronous hash(ahash) ima: define a new hook to measure and appraise a file already in memory kexec: replace call to copy_file_from_fd() with kernel version firmware: replace call to fw_read_file_contents() with kernel version module: replace copy_module_from_fd with kernel version ima: measure and appraise the IMA policy itself ima: require signed IMA policy Documentation/ABI/testing/ima_policy | 2 +- drivers/base/firmware_class.c | 48 fs/exec.c | 93 +++ include/linux/fs.h| 3 + include/linux/ima.h | 17 - include/linux/lsm_hooks.h | 19 + include/linux/security.h | 14 ++-- kernel/kexec_file.c | 72 ++ kernel/module.c | 67 ++-- security/integrity/iint.c | 1 + security/integrity/ima/ima.h | 35 + security/integrity/ima/ima_api.c | 19 ++--- security/integrity/ima/ima_appraise.c | 45 +-- security/integrity/ima/ima_crypto.c | 120 - security/integrity/ima/ima_fs.c | 50 +++- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_main.c | 52 + security/integrity/ima/ima_policy.c | 122 +++--- security/integrity/ima/ima_template.c | 2 - security/integrity/ima/ima_template_lib.c | 1 - security/integrity/integrity.h| 14 ++-- security/security.c | 46 +++ 22 files changed, 540 insertions(+), 304 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH v2 02/11] vfs: define a generic function to read a file from the kernel
For a while it was looked down upon to directly read files from Linux.
These days there exists a few mechanisms in the kernel that do just
this though to load a file into a local buffer. There are minor but
important checks differences on each. This patch set is the first
attempt at resolving some of these differences.
This patch introduces a common function for reading files from the kernel
with the corresponding security post-read hook and function.
Changelog v1:
- To simplify patch review, re-ordered patches
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 53 +++
include/linux/fs.h| 1 +
include/linux/lsm_hooks.h | 9
include/linux/security.h | 7 +++
security/security.c | 8 +++
5 files changed, 78 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index b06623a..6d623c2 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
#include
@@ -831,6 +832,58 @@ int kernel_read(struct file *file, loff_t offset,
EXPORT_SYMBOL(kernel_read);
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+loff_t max_size)
+{
+ loff_t i_size, pos;
+ ssize_t bytes = 0;
+ int ret;
+
+ if (!S_ISREG(file_inode(file)->i_mode))
+ return -EINVAL;
+
+ i_size = i_size_read(file_inode(file));
+ if (max_size > 0 && i_size > max_size)
+ return -EFBIG;
+ if (i_size == 0)
+ return -EINVAL;
+
+ *buf = vmalloc(i_size);
+ if (!*buf)
+ return -ENOMEM;
+
+ pos = 0;
+ while (pos < i_size) {
+ bytes = kernel_read(file, pos, (char *)(*buf) + pos,
+ i_size - pos);
+ if (bytes < 0) {
+ ret = bytes;
+ goto out;
+ }
+
+ if (bytes == 0)
+ break;
+ pos += bytes;
+ }
+
+ if (pos != i_size) {
+ ret = -EIO;
+ goto out;
+ }
+
+ ret = security_kernel_post_read_file(file, *buf, i_size);
+ if (!ret)
+ *size = pos;
+
+out:
+ if (ret < 0) {
+ vfree(*buf);
+ *buf = NULL;
+ }
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t
len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, );
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3aa5142..93ca379 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2527,6 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
+extern int kernel_read_file(struct file *, void **, loff_t *, loff_t);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 71969de..f82631c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -561,6 +561,13 @@
* the kernel module to load. If the module is being loaded from a blob,
* this argument will be NULL.
* Return 0 if permission is granted.
+ * @kernel_post_read_file:
+ * Read a file specified by userspace.
+ * @file contains the file structure pointing to the file being read
+ * by the kernel.
+ * @buf pointer to buffer containing the file contents.
+ * @size length of the file contents.
+ * Return 0 if permission is granted.
* @task_fix_setuid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
@@ -1457,6 +1464,7 @@ union security_list_options {
int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_from_file)(struct file *file);
+ int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size);
int (*task_fix_setuid)(struct cred *new, const struct cred *old,
int flags);
int (*task_setpgid)(struct task_struct *p, pid_t pgid);
@@ -1716,6 +1724,7 @@ struct security_hook_heads {
struct list_head kernel_act_as;
struct list_head kernel_create_files_as;
struct list_head kernel_fw_from_file;
+ struct list_head kernel_post_read_file;
struct list_head kernel_module_request;
struct list_head kernel_module_from_file;
struct list_head task_fix_setuid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 4824a
[RFC PATCH v2 11/11] ima: require signed IMA policy
Require the IMA policy to be signed when additional rules can be added.
Changelog v2:
- add union name "hooks" to fix sparse warning
v1:
- initialize the policy flag
- include IMA_APPRAISE_POLICY in the policy flag
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
security/integrity/ima/ima_policy.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index 7a63760..327e691 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -133,6 +133,10 @@ static struct ima_rule_entry default_appraise_rules[] = {
{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags =
IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = NSFS_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags =
IMA_FSMAGIC},
+#ifdef CONFIG_IMA_WRITE_POLICY
+ {.action = APPRAISE, .hooks.policy_id = POLICY_CHECK,
+ .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
#ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT
{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
#else
@@ -414,9 +418,12 @@ void __init ima_init_policy(void)
for (i = 0; i < appraise_entries; i++) {
list_add_tail(_appraise_rules[i].list,
_default_rules);
+ if (default_appraise_rules[i].hooks.policy_id == POLICY_CHECK)
+ temp_ima_appraise |= IMA_APPRAISE_POLICY;
}
ima_rules = _default_rules;
+ ima_update_policy_flag();
}
/* Make sure we have a valid policy, at least containing some rules. */
--
2.1.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH v2 10/11] ima: measure and appraise the IMA policy itself
This patch adds support for measuring and appraising the IMA policy
itself.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
security/integrity/ima/ima.h| 1 +
security/integrity/ima/ima_fs.c | 9 -
security/integrity/ima/ima_policy.c | 14 --
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fc31ba2..e8f111b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -185,6 +185,7 @@ int ima_policy_show(struct seq_file *m, void *v);
#define IMA_APPRAISE_LOG 0x04
#define IMA_APPRAISE_MODULES 0x08
#define IMA_APPRAISE_FIRMWARE 0x10
+#define IMA_APPRAISE_POLICY0x20
#ifdef CONFIG_IMA_APPRAISE
int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index fe8b16b..57c6b2e 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -325,7 +325,14 @@ static ssize_t ima_write_policy(struct file *file, const
char __user *buf,
if (data[0] == '/')
result = ima_read_policy(data);
- else
+ else if (ima_appraise & IMA_APPRAISE_POLICY) {
+ pr_err("IMA: signed policy required\n");
+ integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
+ "policy_update", "signed policy required",
+ 1, 0);
+ if (ima_appraise & IMA_APPRAISE_ENFORCE)
+ result = -EACCES;
+ } else
result = ima_parse_add_rule(data);
mutex_unlock(_write_mutex);
out_free:
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index dbfd26b..7a63760 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -118,6 +118,7 @@ static struct ima_rule_entry default_measurement_rules[] = {
{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
{.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
.flags = IMA_FUNC},
+ {.action = MEASURE, .hooks.policy_id = POLICY_CHECK, .flags = IMA_FUNC},
};
static struct ima_rule_entry default_appraise_rules[] = {
@@ -618,6 +619,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry
*entry)
entry->hooks.policy_id = FIRMWARE_CHECK;
else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
entry->hooks.policy_id = MODULE_CHECK;
+ else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
+ entry->hooks.policy_id = POLICY_CHECK;
else
result = -EINVAL;
if (!result)
@@ -776,6 +779,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry
*entry)
temp_ima_appraise |= IMA_APPRAISE_MODULES;
else if (entry->hooks.policy_id == FIRMWARE_CHECK)
temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
+ else if (entry->hooks.policy_id == POLICY_CHECK)
+ temp_ima_appraise |= IMA_APPRAISE_POLICY;
audit_log_format(ab, "res=%d", !result);
audit_log_end(ab);
return result;
@@ -862,7 +867,8 @@ static char *mask_tokens[] = {
enum {
func_file = 0, func_mmap, func_bprm,
func_module, func_post,
- func_kexec, func_initramfs, func_firmware
+ func_kexec, func_initramfs, func_firmware,
+ func_policy
};
static char *func_tokens[] = {
@@ -873,7 +879,8 @@ static char *func_tokens[] = {
"POST_SETATTR",
"KEXEC_CHECK",
"INITRAMFS_CHECK",
- "FIRMWARE_CHECK"
+ "FIRMWARE_CHECK",
+ "POLICY_CHECK"
};
void *ima_policy_start(struct seq_file *m, loff_t *pos)
@@ -961,6 +968,9 @@ int ima_policy_show(struct seq_file *m, void *v)
case MODULE_CHECK:
seq_printf(m, pt(Opt_func), ft(func_module));
break;
+ case POLICY_CHECK:
+ seq_printf(m, pt(Opt_func), ft(func_policy));
+ break;
default:
snprintf(tbuf, sizeof(tbuf), "%d",
entry->hooks.func);
--
2.1.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH 5/5] module: replace copy_module_from_fd with kernel version
This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function. Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.
This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file. For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.
This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 4 +++
include/linux/ima.h | 1 +
include/linux/lsm_hooks.h | 8 +
include/linux/security.h | 3 +-
kernel/module.c | 67 ---
security/integrity/ima/ima.h | 1 -
security/integrity/ima/ima_appraise.c | 7
security/integrity/ima/ima_main.c | 5 ++-
security/integrity/ima/ima_policy.c | 16 -
security/integrity/integrity.h| 12 +++
security/security.c | 12 +--
11 files changed, 47 insertions(+), 89 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index f79c845..f251371 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -842,6 +842,10 @@ int kernel_read_file(struct file *file, void **buf, loff_t
*size,
if (!S_ISREG(file_inode(file)->i_mode))
return -EINVAL;
+ ret = security_kernel_read_file(file, policy_id);
+ if (ret)
+ return ret;
+
i_size = i_size_read(file_inode(file));
if (max_size > 0 && i_size > max_size)
return -EFBIG;
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 7cad2e7..969552b 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -18,6 +18,7 @@ enum ima_policy_id {
INITRAMFS_CHECK,
FIRMWARE_CHECK,
POLICY_CHECK,
+ MODULE_CHECK,
IMA_MAX_READ_CHECK
};
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 10baa8f..206a225 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -561,6 +561,12 @@
* the kernel module to load. If the module is being loaded from a blob,
* this argument will be NULL.
* Return 0 if permission is granted.
+ * @kernel_read_file:
+ * Read a file specified by userspace.
+ * @file contains the file structure pointing to the file being read
+ * by the kernel.
+ * @policy_id contains the calling function identifier.
+ * Return 0 if permission is granted.
* @kernel_post_read_file:
* Read a file specified by userspace.
* @file contains the file structure pointing to the file being read
@@ -1465,6 +1471,7 @@ union security_list_options {
int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_from_file)(struct file *file);
+ int (*kernel_read_file)(struct file *file, int policy_id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
int policy_id);
int (*task_fix_setuid)(struct cred *new, const struct cred *old,
@@ -1726,6 +1733,7 @@ struct security_hook_heads {
struct list_head kernel_act_as;
struct list_head kernel_create_files_as;
struct list_head kernel_fw_from_file;
+ struct list_head kernel_read_file;
struct list_head kernel_post_read_file;
struct list_head kernel_module_request;
struct list_head kernel_module_from_file;
diff --git a/include/linux/security.h b/include/linux/security.h
index 51f3bc6..6d005b3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -301,6 +301,7 @@ int security_kernel_act_as(struct cred *new, u32 secid);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
int security_kernel_module_request(char *kmod_name);
int security_kernel_module_from_file(struct file *file);
+int security_kernel_read_file(struct file *file, int policy_id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
int policy_id);
int security_task_fix_setuid(struct cred *new, const struct cred *old,
@@ -857,7 +858,7 @@ static inline int security_kernel_module_request(char
*kmod_name)
return 0;
}
-static inline int security_kernel_module_from_file(struct file *file)
+static inline int security_kernel_read_file(struct file *file, int policy_id)
{
return 0;
}
diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..7398d12 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2665,7 +2665,7 @@ static int copy_module_from_user(const void __user *umod,
unsigned long len,
if (info-&g
[RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version
Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function. Call the existing firmware security hook
from security_kernel_post_read_file() until the LSMs have been converted.
This patch retains the kernel_fw_from_file() hook, but removes the
security_kernel_fw_from_file() function.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
drivers/base/firmware_class.c | 51 +--
include/linux/ima.h | 6 -
include/linux/security.h | 8 +-
security/integrity/ima/ima_main.c | 18 ++
security/security.c | 24 --
5 files changed, 30 insertions(+), 77 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 3ca96a6..4e4e860 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -292,44 +292,10 @@ static const char * const fw_path[] = {
module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
MODULE_PARM_DESC(path, "customized firmware image search path with a higher
priority than default path");
-static int fw_read_file_contents(struct file *file, struct firmware_buf
*fw_buf)
-{
- int size;
- char *buf;
- int rc;
-
- if (!S_ISREG(file_inode(file)->i_mode))
- return -EINVAL;
- size = i_size_read(file_inode(file));
- if (size <= 0)
- return -EINVAL;
- buf = vmalloc(size);
- if (!buf)
- return -ENOMEM;
- rc = kernel_read(file, 0, buf, size);
- if (rc != size) {
- if (rc > 0)
- rc = -EIO;
- goto fail;
- }
- rc = ima_hash_and_process_file(file, buf, size, FIRMWARE_CHECK);
- if (rc)
- goto fail;
-
- rc = security_kernel_fw_from_file(file, buf, size);
- if (rc)
- goto fail;
- fw_buf->data = buf;
- fw_buf->size = size;
- return 0;
-fail:
- vfree(buf);
- return rc;
-}
-
static int fw_get_filesystem_firmware(struct device *device,
struct firmware_buf *buf)
{
+ loff_t size;
int i, len;
int rc = -ENOENT;
char *path;
@@ -355,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device
*device,
file = filp_open(path, O_RDONLY, 0);
if (IS_ERR(file))
continue;
- rc = fw_read_file_contents(file, buf);
+
+ buf->size = 0;
+ rc = kernel_read_file(file, >data, , UINT_MAX,
+ FIRMWARE_CHECK);
fput(file);
if (rc)
dev_warn(device, "firmware, attempted to load %s, but
failed with error %d\n",
path, rc);
- else
+ else {
+ buf->size = (size_t) size;
break;
+ }
}
__putname(path);
@@ -690,8 +661,10 @@ static ssize_t firmware_loading_store(struct device *dev,
dev_err(dev, "%s: map pages failed\n",
__func__);
else
- rc = security_kernel_fw_from_file(NULL,
- fw_buf->data, fw_buf->size);
+ rc = security_kernel_post_read_file(NULL,
+ fw_buf->data,
+ fw_buf->size,
+ FIRMWARE_CHECK);
/*
* Same logic as fw_load_abort, only the DONE bit
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 3790af0..7cad2e7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -27,7 +27,6 @@ extern int ima_file_check(struct file *file, int mask, int
opened);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern int ima_module_check(struct file *file);
-extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
extern int ima_hash_and_process_file(struct file *file,
void *buf, loff_t size,
enum ima_policy_id policy_id);
@@ -58,11 +57,6 @@ static inline int ima_module_check(struct file *file)
return 0;
}
-static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
- return 0;
-}
-
static inline int ima_hash_and_process_file(struct file *file,
void *buf, loff_t size,
enum ima_policy_id policy_id)
diff -
[RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel
In order to measure and appraise files being read by the kernel,
new module and kexec syscalls were defined which include a file
descriptor. Other places in the kernel (eg. firmware, IMA,
sound) also read files.
This patch introduces a common function for reading files from
the kernel with the corresponding security post-read hook and
function.
Changelog:
- Add missing
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 56 +++
include/linux/fs.h| 1 +
include/linux/lsm_hooks.h | 11 ++
include/linux/security.h | 9
security/security.c | 16 ++
5 files changed, 93 insertions(+)
diff --git a/fs/exec.c b/fs/exec.c
index b06623a..3c48a19 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
#include
@@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset,
EXPORT_SYMBOL(kernel_read);
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+loff_t max_size, int policy_id)
+{
+ loff_t i_size, pos;
+ ssize_t bytes = 0;
+ int ret;
+
+ if (!S_ISREG(file_inode(file)->i_mode))
+ return -EINVAL;
+
+ i_size = i_size_read(file_inode(file));
+ if (max_size > 0 && i_size > max_size)
+ return -EFBIG;
+ if (i_size == 0)
+ return -EINVAL;
+
+ *buf = vmalloc(i_size);
+ if (!*buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pos = 0;
+ while (pos < i_size) {
+ bytes = kernel_read(file, pos, (char *)(*buf) + pos,
+ i_size - pos);
+ if (bytes < 0) {
+ ret = bytes;
+ goto out_free;
+ }
+
+ if (bytes == 0)
+ break;
+ pos += bytes;
+ }
+
+ if (pos != i_size) {
+ ret = -EBADF; /* firmware uses -EIO */
+ goto out_free;
+ }
+
+ ret = security_kernel_post_read_file(file, *buf, i_size, policy_id);
+ if (!ret)
+ *size = pos;
+
+out_free:
+ if (ret < 0) {
+ vfree(*buf);
+ *buf = NULL;
+ }
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t
len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, );
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3aa5142..9b1468c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2527,6 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
+extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 71969de..10baa8f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -561,6 +561,14 @@
* the kernel module to load. If the module is being loaded from a blob,
* this argument will be NULL.
* Return 0 if permission is granted.
+ * @kernel_post_read_file:
+ * Read a file specified by userspace.
+ * @file contains the file structure pointing to the file being read
+ * by the kernel.
+ * @buf pointer to buffer containing the file contents.
+ * @size length of the file contents.
+ * @policy_id contains the calling function identifier.
+ * Return 0 if permission is granted.
* @task_fix_setuid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
@@ -1457,6 +1465,8 @@ union security_list_options {
int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
int (*kernel_module_request)(char *kmod_name);
int (*kernel_module_from_file)(struct file *file);
+ int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
+int policy_id);
int (*task_fix_setuid)(struct cred *new, const struct cred *old,
int flags);
int (*task_setpgid)(struct task_struct *p, pid_t pgid);
@@ -1716,6 +1726,7 @@ struct security_hook_heads {
struct list_head kernel_act_as;
struct list_head kernel_create_files_as;
struct list_head kernel_fw_from_file;
+ struct list_head kernel_post_read_file;
struct list_head kernel_module_request;
struct list_head kernel_module_from_file;
struct list_
[RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1)
For a while it was looked down upon to directly read files from Linux. These days there exists a few mechanisms in the kernel that do just this though to load a file into a local buffer. There are minor but important checks differences on each, we should take all the best practices from each of them, generalize them and make all places in the kernel that read a file use it.[1] One difference is the method for opening the file. In some cases we have a file, while in other cases we have a pathname or a file descriptor. Another difference is the security hook calls, or lack of them. In some versions there is a post file read hook, while in others there is a pre file read hook. This patch set is the first attempt at resolving these differences. It does not attempt to merge the different methods of opening a file, but defines a single common kernel file read function with two wrappers. Although this patch set defines two new security hooks for pre and post file read, it does not attempt to merge the existing security hooks. That is left as future work. These patches are based on top of the "ima: measuring/appraising files read by the kernel". The latest version of these patches can be found in the next-kernel-read branch of: git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git Mimi Zohar (5): vfs: define a generic function to read a file from the kernel firmware: replace call to fw_read_file_contents() with kernel version kexec: replace call to copy_file_from_fd() with kernel version ima: replace call to integrity_read_file() with kernel version module: replace copy_module_from_fd with kernel version drivers/base/firmware_class.c | 51 +-- fs/exec.c | 96 +++ include/linux/fs.h| 3 ++ include/linux/ima.h | 7 +-- include/linux/lsm_hooks.h | 19 +++ include/linux/security.h | 14 +++-- kernel/kexec_file.c | 76 +++ kernel/module.c | 67 +++- security/integrity/ima/ima.h | 1 - security/integrity/ima/ima_appraise.c | 7 --- security/integrity/ima/ima_fs.c | 15 +++--- security/integrity/ima/ima_main.c | 21 security/integrity/ima/ima_policy.c | 16 +++--- security/integrity/integrity.h| 12 ++--- security/security.c | 46 - 15 files changed, 217 insertions(+), 234 deletions(-) -- 2.1.0 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH 3/5] kexec: replace call to copy_file_from_fd() with kernel version
This patch defines kernel_read_file_from_fd(), a wrapper for the VFS
common kernel_read_file(), and replaces the kexec copy_file_from_fd()
calls with the kernel_read_file_from_fd() wrapper.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 15 +++
include/linux/fs.h | 1 +
kernel/kexec_file.c | 76 +
3 files changed, 23 insertions(+), 69 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 3c48a19..4ad2fca 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -887,6 +887,21 @@ out:
}
EXPORT_SYMBOL_GPL(kernel_read_file);
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+int policy_id)
+{
+ struct fd f = fdget(fd);
+ int ret = -ENOEXEC;
+
+ if (!f.file)
+ goto out;
+
+ ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+ fdput(f);
+ return ret;
+}
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t
len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, );
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9b1468c..9642623 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 81d20e8..f7c3ce4 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -34,69 +34,6 @@ size_t __weak kexec_purgatory_size = 0;
static int kexec_calculate_store_digests(struct kimage *image);
-static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len,
-enum ima_policy_id policy_id)
-{
- struct fd f = fdget(fd);
- int ret;
- struct kstat stat;
- loff_t pos;
- ssize_t bytes = 0;
-
- if (!f.file)
- return -EBADF;
-
- ret = vfs_getattr(>f_path, );
- if (ret)
- goto out;
-
- if (stat.size > INT_MAX) {
- ret = -EFBIG;
- goto out;
- }
-
- /* Don't hand 0 to vmalloc, it whines. */
- if (stat.size == 0) {
- ret = -EINVAL;
- goto out;
- }
-
- *buf = vmalloc(stat.size);
- if (!*buf) {
- ret = -ENOMEM;
- goto out;
- }
-
- pos = 0;
- while (pos < stat.size) {
- bytes = kernel_read(f.file, pos, (char *)(*buf) + pos,
- stat.size - pos);
- if (bytes < 0) {
- ret = bytes;
- goto out_free;
- }
-
- if (bytes == 0)
- break;
- pos += bytes;
- }
-
- if (pos != stat.size)
- ret = -EBADF;
-
- ret = ima_hash_and_process_file(f.file, *buf, stat.size, policy_id);
- if (!ret)
- *buf_len = pos;
-out_free:
- if (ret < 0) {
- vfree(*buf);
- *buf = NULL;
- }
-out:
- fdput(f);
- return ret;
-}
-
/* Architectures can provide this probe function */
int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
unsigned long buf_len)
@@ -185,16 +122,17 @@ kimage_file_prepare_segments(struct kimage *image, int
kernel_fd, int initrd_fd,
{
int ret = 0;
void *ldata;
+ loff_t size;
- ret = copy_file_from_fd(kernel_fd, >kernel_buf,
- >kernel_buf_len, KEXEC_CHECK);
+ ret = kernel_read_file_from_fd(kernel_fd, >kernel_buf,
+ , INT_MAX, KEXEC_CHECK);
if (ret)
return ret;
+ image->kernel_buf_len = size;
/* Call arch image probe handlers */
ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
image->kernel_buf_len);
-
if (ret)
goto out;
@@ -209,11 +147,11 @@ kimage_file_prepare_segments(struct kimage *image, int
kernel_fd, int initrd_fd,
#endif
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
- ret = copy_file_from_fd(initrd_fd, >initrd_buf,
- >initrd_buf_len,
- INITRAMFS_CHECK);
+ ret = kernel_read_file_from_fd(initrd_fd, >initrd_buf,
+
[RFC PATCH 4/5] ima: replace call to integrity_read_file() with kernel version
This patch defines kernel_read_file_from_path(), a wrapper for the VFS
common kernel_read_file(), and replaces the integrity_read_file() with
a call to the kernel_read_file_from_path() wrapper.
Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
---
fs/exec.c | 21 +
include/linux/fs.h | 1 +
security/integrity/ima/ima_fs.c | 15 +--
3 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 4ad2fca..f79c845 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -902,6 +902,27 @@ out:
return ret;
}
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+ loff_t max_size, int policy_id)
+{
+ struct file *file;
+ int ret;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file)) {
+ ret = PTR_ERR(file);
+ pr_err("Unable to open file: %s (%d)", path, ret);
+ return ret;
+ }
+
+ ret = kernel_read_file(file, buf, size, max_size, policy_id);
+ fput(file);
+ return ret;
+}
+
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t
len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, );
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9642623..79b1172 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2529,6 +2529,7 @@ extern int do_pipe_flags(int *, int);
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t, int);
extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
extern struct file * open_exec(const char *);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 685fdca..80bc30b 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include "ima.h"
@@ -260,20 +261,22 @@ static const struct file_operations
ima_ascii_measurements_ops = {
static ssize_t ima_read_policy(char *path)
{
- char *data, *datap;
- int rc, size, pathlen = strlen(path);
+ void *data;
+ char *datap;
+ loff_t size;
+ int rc, pathlen = strlen(path);
+
char *p;
/* remove \n */
datap = path;
strsep(, "\n");
- rc = integrity_read_file(path, , POLICY_CHECK);
+ rc = kernel_read_file_from_path(path, , , 0, POLICY_CHECK);
if (rc < 0)
return rc;
- size = rc;
- datap = data;
+ datap = (char *)data;
while (size > 0 && (p = strsep(, "\n"))) {
pr_debug("rule: %s\n", p);
rc = ima_parse_add_rule(p);
@@ -282,7 +285,7 @@ static ssize_t ima_read_policy(char *path)
size -= rc;
}
- kfree(data);
+ vfree(data);
if (rc < 0)
return rc;
else if (size)
--
2.1.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH 1/5] vfs: define a generic function to read a file from the kernel
On Fri, 2016-01-08 at 12:24 -0800, Kees Cook wrote:
> On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar <zo...@linux.vnet.ibm.com> wrote:
> > In order to measure and appraise files being read by the kernel,
> > new module and kexec syscalls were defined which include a file
> > descriptor. Other places in the kernel (eg. firmware, IMA,
> > sound) also read files.
> >
> > This patch introduces a common function for reading files from
> > the kernel with the corresponding security post-read hook and
> > function.
> >
> > Changelog:
> > - Add missing
> >
> > Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
> > ---
> > fs/exec.c | 56
> > +++
> > include/linux/fs.h| 1 +
> > include/linux/lsm_hooks.h | 11 ++
> > include/linux/security.h | 9
> > security/security.c | 16 ++
> > 5 files changed, 93 insertions(+)
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > index b06623a..3c48a19 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -56,6 +56,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -831,6 +832,61 @@ int kernel_read(struct file *file, loff_t offset,
> >
> > EXPORT_SYMBOL(kernel_read);
> >
> > +int kernel_read_file(struct file *file, void **buf, loff_t *size,
> > +loff_t max_size, int policy_id)
> > +{
> > + loff_t i_size, pos;
> > + ssize_t bytes = 0;
> > + int ret;
> > +
> > + if (!S_ISREG(file_inode(file)->i_mode))
> > + return -EINVAL;
> > +
> > + i_size = i_size_read(file_inode(file));
> > + if (max_size > 0 && i_size > max_size)
> > + return -EFBIG;
> > + if (i_size == 0)
> > + return -EINVAL;
> > +
> > + *buf = vmalloc(i_size);
>
> This could get very large -- what risks do we have to system stability
> here? Having userspace able to trigger such a massive allocation could
> be a problem. The firmware loader was limited to MAX_INT...
The different callers allowed different sizes. Instead of hard coding
the max size for all callers, the third parameter of kernel_file_read is
the caller max_size.
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH 0/5] vfs: support for a common kernel file loader (step 1)
On Fri, 2016-01-08 at 14:21 -0500, Mimi Zohar wrote: > For a while it was looked down upon to directly read files from Linux. > These days there exists a few mechanisms in the kernel that do just this > though to load a file into a local buffer. There are minor but important > checks differences on each, we should take all the best practices from > each of them, generalize them and make all places in the kernel that > read a file use it.[1] > > One difference is the method for opening the file. In some cases we > have a file, while in other cases we have a pathname or a file descriptor. > > Another difference is the security hook calls, or lack of them. In > some versions there is a post file read hook, while in others there > is a pre file read hook. > > This patch set is the first attempt at resolving these differences. It > does not attempt to merge the different methods of opening a file, but > defines a single common kernel file read function with two wrappers. > Although this patch set defines two new security hooks for pre and post > file read, it does not attempt to merge the existing security hooks. > That is left as future work. > > These patches are based on top of the "ima: measuring/appraising files > read by the kernel". The latest version of these patches can be found > in the next-kernel-read branch of: > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git [1] Taken from Luis Rodriguez's wiki - http://kernelnewbies.org/KernelProjects/common-kernel-loader Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH 2/5] firmware: replace call to fw_read_file_contents() with kernel version
On Fri, 2016-01-08 at 12:26 -0800, Kees Cook wrote:
> On Fri, Jan 8, 2016 at 11:22 AM, Mimi Zohar <zo...@linux.vnet.ibm.com> wrote:
> > Replace fw_read_file_contents() for reading a file with the common VFS
> > kernel_read_file() function. Call the existing firmware security hook
> > from security_kernel_post_read_file() until the LSMs have been converted.
> >
> > This patch retains the kernel_fw_from_file() hook, but removes the
> > security_kernel_fw_from_file() function.
> >
> > Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com>
> > ---
> > drivers/base/firmware_class.c | 51
> > +--
> > include/linux/ima.h | 6 -
> > include/linux/security.h | 8 +-
> > security/integrity/ima/ima_main.c | 18 ++
> > security/security.c | 24 --
> > 5 files changed, 30 insertions(+), 77 deletions(-)
> >
> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> > index 3ca96a6..4e4e860 100644
> > --- a/drivers/base/firmware_class.c
> > +++ b/drivers/base/firmware_class.c
> > @@ -292,44 +292,10 @@ static const char * const fw_path[] = {
> > module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
> > MODULE_PARM_DESC(path, "customized firmware image search path with a
> > higher priority than default path");
> >
> > -static int fw_read_file_contents(struct file *file, struct firmware_buf
> > *fw_buf)
> > -{
> > - int size;
> > - char *buf;
> > - int rc;
> > -
> > - if (!S_ISREG(file_inode(file)->i_mode))
> > - return -EINVAL;
> > - size = i_size_read(file_inode(file));
> > - if (size <= 0)
> > - return -EINVAL;
> > - buf = vmalloc(size);
> > - if (!buf)
> > - return -ENOMEM;
> > - rc = kernel_read(file, 0, buf, size);
> > - if (rc != size) {
> > - if (rc > 0)
> > - rc = -EIO;
> > - goto fail;
> > - }
> > - rc = ima_hash_and_process_file(file, buf, size, FIRMWARE_CHECK);
> > - if (rc)
> > - goto fail;
> > -
> > - rc = security_kernel_fw_from_file(file, buf, size);
> > - if (rc)
> > - goto fail;
> > - fw_buf->data = buf;
> > - fw_buf->size = size;
> > - return 0;
> > -fail:
> > - vfree(buf);
> > - return rc;
> > -}
> > -
> > static int fw_get_filesystem_firmware(struct device *device,
> >struct firmware_buf *buf)
> > {
> > + loff_t size;
> > int i, len;
> > int rc = -ENOENT;
> > char *path;
> > @@ -355,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device
> > *device,
> > file = filp_open(path, O_RDONLY, 0);
> > if (IS_ERR(file))
> > continue;
> > - rc = fw_read_file_contents(file, buf);
> > +
> > + buf->size = 0;
> > + rc = kernel_read_file(file, >data, , UINT_MAX,
>
> Strictly speaking, the originally code would max at INT_MAX, no UINT_MAX.
hm, I must have taken it from firmware_buf->size, which is defined as
size_t (unsigned). Thanks for the correction.
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Tue, 2015-12-29 at 16:21 +0800, Dave Young wrote: > Hi, Mimi > > On 12/28/15 at 07:51am, Mimi Zohar wrote: > > On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote: > > > On 12/25/15 at 09:45am, Mimi Zohar wrote: > > > > IMA calculates the file hash, in this case, based on the buffer > > > > contents. The hash is calculated once and used for both measurement > > > > and appraisal. If the file integrity appraisal fails (eg. hash > > > > comparison or signature failure), IMA prevents the kexec files from > > > > being used. > > > > > > > > > > Ok, thanks for the explanatioin. But I have another question, why do we > > > need a special hook for KEXEC? Shouldn't all files use same way to do the > > > measurement and appraisal? > > > > "By all files" are you referring to all files read by the kernel or all > > files opened, executed or mmapped by the system? > > Hmm, I means any kind of files read by the kernel. > > > > > Currently IMA allocates a page sized buffer, reads a file a page chunk > > at a time calculating the file hash as it does so, and then frees the > > buffer before returning to the caller. This method of calculating the > > file hash is used for measuring and appraising files opened > > (FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the > > system. > > > > This patch set addresses files being read by kernel. A single new > > generic hook named ima_hash_and_process_file() is defined to not only > > measure and appraise the kexec image and initramfs, but firmware and the > > IMA policy. As we identify other places that the kernel is reading > > files, this hook would be called in those places as well. > > What I can not understand is why IMA need know the caller information and > why cann't introduce a generic interface. kexec and firmware and other > caller all read files, so a common file based interface should be better? The next patch set will define a common function for reading files by the kernel. Luis set up a wiki http://kernelnewbies.org/KernelProjects/common-kernel-loader with some details. This patch set defines a generic interface for measuring and appraising files being read by the kernel, with the ability to define a policy based on the caller information. For the details on expressing a policy, refer to Documentation/ABI/testing/ima-policy. For example, the new rules could be expressed like: measure func=KEXEC_CHECK appraise func=KEXEC_CHECK appraise_type=imasig # measure func=INITRAMFS_CHECK appraise func=INITRAMFS_CHECK appraise_type=imasig # measure func=FIRMWARE_CHECK appraise func=FIRMWARE_CHECK appraise_type=imasig # measure func=POLICY_CHECK appraise func=POLICY_CHECK appraise_type=imasig This policy flexibility is needed at least until all files come from software providers with file signatures. (RPM has been modified to include file signatures.) Even then, in terms of kexec, some distros generate the initramfs on the target host and, therefore, can not sign the initramfs. The local user could, however, sign the initramfs on their own system. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Tue, 2015-12-29 at 07:06 -0500, Mimi Zohar wrote: > On Tue, 2015-12-29 at 16:21 +0800, Dave Young wrote: > This policy flexibility is needed at least until all files come from > software providers with file signatures. (RPM has been modified to > include file signatures.) Even then, in terms of kexec, some distros > generate the initramfs on the target host and, therefore, can not sign > the initramfs. The local user could, however, sign the initramfs on > their own system. Sorry, instead of "local user" the "local system/host owner" would be more appropriate. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Mon, 2015-12-28 at 16:29 +0200, Petko Manolov wrote: > On 15-12-28 07:51:15, Mimi Zohar wrote: > > On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote: > > > On 12/25/15 at 09:45am, Mimi Zohar wrote: > > > > IMA calculates the file hash, in this case, based on the buffer > > > > contents. The hash is calculated once and used for both measurement > > > > and appraisal. If the file integrity appraisal fails (eg. hash > > > > comparison or signature failure), IMA prevents the kexec files from > > > > being used. > > > > > > > > > > Ok, thanks for the explanatioin. But I have another question, why do we > > > need a special hook for KEXEC? Shouldn't all files use same way to do the > > > measurement and appraisal? > > > > "By all files" are you referring to all files read by the kernel or all > > files opened, executed or mmapped by the system? > > > > Currently IMA allocates a page sized buffer, reads a file a page chunk > > at a time calculating the file hash as it does so, and then frees the > > buffer before returning to the caller. This method of calculating the > > I kind of wonder isn't it possible to optimize the file read? If the file is > relatively small (a few megabytes, for example) it will fit into any modern > system's memory. At least those that cares to run IMA, i mean. > > Fetching file page by page is a slow process even though the BIO subsystem > reads > larger chunks off the real storage devices. Has anyone done a benchmark test? Dmitry recently added asynchronous hash (ahash) support, which allows HW crypto acceleration, for calculating the file hash. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote: > On 12/25/15 at 09:45am, Mimi Zohar wrote: > > IMA calculates the file hash, in this case, based on the buffer > > contents. The hash is calculated once and used for both measurement > > and appraisal. If the file integrity appraisal fails (eg. hash > > comparison or signature failure), IMA prevents the kexec files from > > being used. > > > > Ok, thanks for the explanatioin. But I have another question, why do we > need a special hook for KEXEC? Shouldn't all files use same way to do the > measurement and appraisal? "By all files" are you referring to all files read by the kernel or all files opened, executed or mmapped by the system? Currently IMA allocates a page sized buffer, reads a file a page chunk at a time calculating the file hash as it does so, and then frees the buffer before returning to the caller. This method of calculating the file hash is used for measuring and appraising files opened (FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the system. This patch set addresses files being read by kernel. A single new generic hook named ima_hash_and_process_file() is defined to not only measure and appraise the kexec image and initramfs, but firmware and the IMA policy. As we identify other places that the kernel is reading files, this hook would be called in those places as well. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote: > On 12/25/15 at 09:45am, Mimi Zohar wrote: > > IMA calculates the file hash, in this case, based on the buffer > > contents. The hash is calculated once and used for both measurement > > and appraisal. If the file integrity appraisal fails (eg. hash > > comparison or signature failure), IMA prevents the kexec files from > > being used. > > > > Ok, thanks for the explanatioin. But I have another question, why do we > need a special hook for KEXEC? Shouldn't all files use same way to do the > measurement and appraisal? "By all files" are you referring to all files read by the kernel or all files opened, executed or mmapped by the system? Currently IMA allocates a page sized buffer, reads a file a page chunk at a time calculating the file hash as it does so, and then frees the buffer before returning to the caller. This method of calculating the file hash is used for measuring and appraising files opened (FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the system. This patch set addresses files being read by kernel. A single new generic hook named ima_hash_and_process_file() is defined to not only measure and appraise the kexec image and initramfs, but firmware and the IMA policy. As we identify other places that the kernel is reading files, this hook would be called in those places as well. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Mon, 2015-12-28 at 16:59 +0200, Petko Manolov wrote: > On 15-12-28 09:42:22, Mimi Zohar wrote: > > On Mon, 2015-12-28 at 16:29 +0200, Petko Manolov wrote: > > > > > > I kind of wonder isn't it possible to optimize the file read? If the > > > file > > > is relatively small (a few megabytes, for example) it will fit into any > > > modern system's memory. At least those that cares to run IMA, i mean. > > > > > > Fetching file page by page is a slow process even though the BIO > > > subsystem > > > reads larger chunks off the real storage devices. Has anyone done a > > > benchmark test? > > > > Dmitry recently added asynchronous hash (ahash) support, which allows HW > > crypto acceleration, for calculating the file hash. > > This is nice. However, i was referring to reading a file page by page vs > larger > (a couple of megabytes) chunks. Is this also covered by the ahash support? Yes, basically it attempts to allocate a buffer for the entire file. On failure, ahash attempts to allocate two buffers larger than PAGE_SIZE, but falls back to using just PAGE_SIZE. Refer to ima_alloc_pages() for a full description. When two buffers are allocated, while waiting for one hash to complete, the subsequent file read is read into the other buffer. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
On Fri, 2015-12-25 at 13:33 +0800, Dave Young wrote: > Hi, Mimi > > CCing kexec list, not all kexec people subscribed to IMA list. > I just subscribed to it since Vivek CCed me last time about the V1 of this > series. Thanks! > On 12/23/15 at 06:55pm, Mimi Zohar wrote: > > This patch defines a new IMA hook ima_hash_and_process_file() for > > measuring and appraising files read by the kernel. The caller loads > > the file into memory before calling this function, which calculates > > the hash followed by the normal IMA policy based processing. > > > > Two new IMA policy functions named KEXEC_CHECK and INITRAMFS_CHECK > > are defined for measuring, appraising or auditing the kexec image > > and initramfs. > > Could you help us understand why do we need it first. IMA can be viewed as extending secure and trusted boot to the running system in a uniform and consistent manner. As files are accessed, based on policy, IMA measures them, appends the file measurements to the running measurement list (/ima/ascii_runtime_measurements) and appraises the file's integrity, based on either the file's hash or signature, which are stored as extended attributes in "security.ima". There are still a couple of file measurement and appraisal gaps that need to be closed. > I think I do not really understand the purpose of the IMA handling > about kexec kernel and initramfs. One of those measurement and appraisal gaps are files that are read by the kernel, like the kexec image and initramfs. [There is a lot of code duplication in the kernel for reading a file and verifying its signature. Each place does it just a bit differently than the other. I'm working with Luis Rodriguez on defining a single, common function - https://lkml.org/lkml/2015/12/21/478.] > * Does the files in disk space have already contains some hash values > and when kernel load it IMA functions will do some checking? But seems I do > not > see such handling.. IMA sits on a number of the LSM hooks, where they exist, and in other places defines its own hook. This patch set defines a new IMA hook for measuring and appraising files being read by the kernel. > * Does it try to calculate the hash of the file buffer after copying, IMA calculates the file hash, in this case, based on the buffer contents. The hash is calculated once and used for both measurement and appraisal. If the file integrity appraisal fails (eg. hash comparison or signature failure), IMA prevents the kexec files from being used. > and IMA will avoid future modification based on the hash calculated? > If this is the purpose I think it should be wrong because kexe file buffers > will be freed at the end of kexec_file_load syscall. The ima_hash_and _process_file() call calculates the file's hash, adds the hash to the IMA measurement list and appraises the file's integrity. On integrity failure, in this case, it prevents the kexec files from being used, causing the buffers to be freed. Mimi > > > > Changelog v2: > > - Calculate the file hash from the in memory buffer (suggested by Dave > > Young) > > - Rename ima_read_and_process_file() to ima_hash_and_process_file() > > - replace individual case statements with range: > > KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1 > > v1: > > - Instead of ima_read_and_process_file() allocating memory, the caller > > allocates and frees the memory. > > - Moved the kexec measurement/appraisal call to copy_file_from_fd(). The > > same call now measures and appraises both the kexec image and initramfs. > > > > Signed-off-by: Mimi Zohar <zo...@linux.vnet.ibm.com> > > --- > > Documentation/ABI/testing/ima_policy | 2 +- > > include/linux/ima.h | 16 ++ > > kernel/kexec_file.c | 24 > > security/integrity/iint.c | 1 + > > security/integrity/ima/ima.h | 21 -- > > security/integrity/ima/ima_api.c | 6 +++-- > > security/integrity/ima/ima_appraise.c | 11 -- > > security/integrity/ima/ima_main.c | 41 > > --- > > security/integrity/ima/ima_policy.c | 38 > > security/integrity/integrity.h| 7 -- > > 10 files changed, 127 insertions(+), 40 deletions(-) > > > > diff --git a/Documentation/ABI/testing/ima_policy > > b/Documentation/ABI/testing/ima_policy > > index 0a378a8..e80f767 100644 > > --- a/Documentation/ABI/testing/ima_policy > > +++ b/Documentation/ABI/testing/ima_policy > > @@ -26,7 +26,7 @@ Description: > > option: [[appraise_type=]] [permit_directio] > > &g
Re: [PATCH 04/16] integrity: Allow digital signature verification with a given keyring ptr
On Tue, 2013-09-10 at 17:44 -0400, Vivek Goyal wrote: Currently digital signature verification code assumes that it can be used only with 3 keyrings. IMA, EVM and MODULE keyring. Provide another variant where one can pass in a pointer to keyring (struct key *), and integrity code can try to find key in that keyring and verify signature. This will be useful at two places. - elf binary loader can use system keyring and call into integrity subsystem for signature verification. - In later patches I am extending keyctl() to allow signature of a user buffer against specified keyring. That logic can make use of this code too. Signed-off-by: Vivek Goyal vgo...@redhat.com My original thought was to use the system keyring, in lieu of having the multiple keyrings. Unfortunately, the scope of a key's usage needs to be limited, which can not be done safely with a single keyring. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 01/12] Security: Add CAP_COMPROMISE_KERNEL
On Wed, 2013-03-20 at 18:12 +, Matthew Garrett wrote: On Wed, 2013-03-20 at 14:01 -0400, Mimi Zohar wrote: Sorry, I'm not sure to which work you're referring. If you're referring to Dmitry's initramfs with digital signature protection patches, then we're speaking about enforcing integrity, not MAC security. Well, in the absence of hardcoded in-kernel policy, there needs to be some mechanism for ensuring the integrity of a policy. Shipping a signed policy initramfs fragment and having any Secure Boot bootloaders pass a flag in bootparams indicating that the kernel should panic if that fragment isn't present would seem to be the easiest way of doing that. Or have I misunderstood the question? Ok, I was confused by the term fragmented initramfs. So once you have verified the early fragmented initramfs signature, this initramfs will load the trusted public keys and could also load the MAC policy. (I realize that dracut is currently loading the MAC policy, not the initramfs.) The MAC policy would then be trusted, right? Could we then use the LSM labels for defining an integrity policy for kexec? thanks, Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: Kdump with signed images
On Wed, 2012-11-14 at 21:09 -0800, Eric W. Biederman wrote: Vivek Goyal vgo...@redhat.com writes: On Thu, Nov 08, 2012 at 01:03:17PM -0800, Eric W. Biederman wrote: Vivek Goyal vgo...@redhat.com writes: On Thu, Nov 08, 2012 at 02:40:50PM -0500, Vivek Goyal wrote: On Tue, Nov 06, 2012 at 03:51:59PM -0800, Eric W. Biederman wrote: [..] Thnking more about executable signature verification, I have another question. While verifyign the signature, we will have to read the whole executable in memory. That sounds bad as we are in kernel mode and will not be killed and if sombody is trying to execute a malformed exceptionally large executable, system will start killing other processess. We can potentially lock all the memory in kernel just by trying to execute a signed huge executable. Not good. Also, even if we try to read in whole executable, can't an hacker modify pages in swap disk and then they will be faulted back in and bingo hacker is running its unsigned code. (assuming root has been compromised otherwise why do we have to do all this exercise). You make a decent case for an implicit mlockall(MCL_FUTURE) being required of signed executables, that are going to be granted privileges based on signature verification. implicity lockall for signed executables sounds reasonable to avoid the swap hack. As for size if the executable won't fit in memory, there is no point in checking the signature. Well I am worried about malformed executables. One can sign a huge executable (which is never meant to run successfully) and cause all kind of memory issues. Good point what to do with executables with invalid sigantures. From another reply it sounded like one of the bits of IMA/EVM had already addressed part of that. With IMA-appraisal enabled (enforcing mode), it would not be executed. Can we first look at the signature, decrypt it using certificates in kernel ring, and if we find out that executable was signed by any of the certificates, only then we go on to read in whole executable and try to calculate the digest. May be at the time of signing we can put a string, say LINUX, along with digest and then sing/encrypt it. Upon decryption we can check if LINUX is there and if yes, we know it was signed by the certifcate loaded in kernel and then go on to load the full executable and calculate digest. Not sure if above is doable or not but if it is, it might reduce the risk significantly as we will not try to integrity verify executables not signed by genuine certificates. Known plaintext in the signed blob should allow that. I would be very careful with that because it sounds like the kind of thing that opens you up to plain-text attacks, but that is mostly my parania and lack of experience speaking. Although IMA (measurement and attestation), IMA-appraisal (local integrity enforcement), and IMA auditing (logging hashes) can be enabled individually, if any of these functions are enabled, then assuming the file is in the IMA policy, the file will be hashed. Remember if all else fails, measurement and attestation is your last line of defense, for detecting if your system has been compromised. Mimi It should be fairly straight forward to make the signature checking process preemptable and killable. hmm..., not sure how to do this. Will have to read more code to understand process killing and see what can I do this while I am in kernel mode and I possibly might have done kernel memory allocations using vmalloc()/kmalloc() etc. Well basically it is matter of using the killable version of waits returning an error code as you unwind, and eventually either force_sig(SIGKILL) or do_exit(). There are a lot of times where you can support SIGKILL and just cause the process to exit where you can't handle signals. Eric ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: Kdump with signed images
On Thu, 2012-11-08 at 14:40 -0500, Vivek Goyal wrote: On Tue, Nov 06, 2012 at 03:51:59PM -0800, Eric W. Biederman wrote: [..] Thnking more about executable signature verification, I have another question. While verifyign the signature, we will have to read the whole executable in memory. That sounds bad as we are in kernel mode and will not be killed and if sombody is trying to execute a malformed exceptionally large executable, system will start killing other processess. We can potentially lock all the memory in kernel just by trying to execute a signed huge executable. Not good. I was looking at IMA and they seem to be using kernel_read() for reading page in and update digest. IIUC, that means page is read from disk, brought in cache and if needed will be read back from disk. But that means hacker can try to do some timing tricks and try to replace disk image after signature verification and run unsigned program. For the reason you mentioned, the signature verification is deferred to bprm, after the executable has been locked from modification. Any subsequent changes to the file would cause the file to be re-appraised. The goal of EVM/IMA-appraisal is to detect file tampering and enforce file data/metadata integrity. If EVM/IMA-appraisal fail, then as a last resort, we fall back and rely on IMA measurement/attestation at least to detect it. Mimi So how do we go about it. Neither of the approaches sound appealing to me. Thanks Vivek ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: Kdump with signed images
On Fri, 2012-10-26 at 03:39 +0100, Matthew Garrett wrote: On Thu, Oct 25, 2012 at 09:15:58PM -0400, Mimi Zohar wrote: On a running system, the package installer, after verifying the package integrity, would install each file with the associated 'security.ima' extended attribute. The 'security.evm' digital signature would be installed with an HMAC, calculated using a system unique key. The idea isn't to prevent /sbin/kexec from being modified after installation - it's to prevent it from being possible to install a system that has a modified /sbin/kexec. Understood. Leaving any part of this up to the package installer means that it doesn't solve the problem we're trying to solve here. It must be impossible for the kernel to launch any /sbin/kexec that hasn't been signed by a trusted key that's been built into the kernel, With Dmitry's patch 5e0d1a4 ima: added policy support for security.ima type, or something similar, we can force 'security.ima' to a specific type, in this case, a digital signature. With that patch, this shouldn't be a problem. and it must be impossible for anything other than /sbin/kexec to make the kexec system call. Permission is a MAC issue. :) thanks, Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: Kdump with signed images
On Fri, 2012-10-26 at 19:19 +0100, Matthew Garrett wrote: On Fri, Oct 26, 2012 at 01:59:34PM -0400, Mimi Zohar wrote: On Fri, 2012-10-26 at 03:39 +0100, Matthew Garrett wrote: and it must be impossible for anything other than /sbin/kexec to make the kexec system call. Permission is a MAC issue. :) It's a MAC issue that has to be implemented in the kernel. We can't depend on userspace loading any kind of policy. Still a MAC issue, that problably could be addressed by capabilities. I suggest you post this issue on the LSM mailing list. Please cc: Serge, as the capabilities maintainer. thanks, Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: Kdump with signed images
On Wed, 2012-10-24 at 13:36 -0400, Vivek Goyal wrote: On Tue, Oct 23, 2012 at 09:19:27AM -0700, Eric W. Biederman wrote: Vivek Goyal vgo...@redhat.com writes: On Tue, Oct 23, 2012 at 09:18:54AM -0400, Vivek Goyal wrote: [..] There are 3 options for trusting /sbin/kexec. There are IMA and EMA, and it is conceivable to have ELF note sections with signatures for executables. Can you please tell more about what is EMA and IMA. I did quick google and could not find much. That should have been EVM and IMA. Look under security/integrity/. I don't know much about them but they appear to be security modules with a focus on verifying checksum or perhaps encrypted hashes of executables are consistent. I will do some quick search there and I see if I can understand something. Ok, I quickly went through following paper. http://mirror.transact.net.au/sourceforge/l/project/li/linux-ima/linux-ima/Integrity_overview.pdf So it looks like that IMA can store the hashes of files and at execute time ensure those hashes are unchanged to protect against the possibility of modification of files. IMA-appraisal originally was hashed based, but Dmitry Kasatkin added digital signature support. Both have been upstreamed. But what about creation of a new program which can call kexec_load() and execute an unsigned kernel. Doesn't look like that will be prevented using IMA. Assuming the IMA policy syntax is updated to require 'security.ima' to contain a digital signature, then it is only a question of protecting the _ima and _evm keyrings. (Dmitry has such a patch waiting to be reviewed.) So the new program would have to be vetted by someone trusted. Whole idea behind UEFI secure boot seems to be that all signing happens outside the running system and now only signed code can run with higher priviliges. No. UEFI secure boot has absolutely nothing todo with this. UEFI secure boot is about not being able to hijack the code EFI runs directly. Full stop. Some people would like to implment a security policy that says you can't boot an untrusted version of windows from linux if you have booted with UEFI secure boot, so they don't get their bootloader signatures revoked by microsoft. A security model relying on Microsoft's key is totally uniteresting to me. Either signing at the UEFI level is of no use or Microsofts key will fall again to the combined assult of every cracker and every governmental dirty cyber ops division attacking it. Not to mention that Microsoft has little incentive to keep linux booting. I think it is reasonable to be able to support a policy where we can't boot unsigned versions of Microsoft windows. However beyond being able to exclude booting windows being one criteria for our policy mechanism please don't even start to justify things with that ridiculous security policy even indirectly. IMA seems to be only protecting against only making sure existing binaries are not modifed but it does not seem to prevent against installation of new binaries and these binaries take advantage of kexec system call to load an unsigned kernel. The IMA/IMA-appraisal policy dictates what needs to be appraised. The default ima-appraisal policy appraises all files owned by root. I believe you can combine IMA with EVM signed security attributes where the EVM signing key is offline, and the verification key is in the kernel. The combination of IMA and EVM gets very close to being able to sign executables offline and be able to update them. [ Again CCing lkml and IMA/EVM folks ] After little reading, my understanding is EVM also does not support offline signing. http://sourceforge.net/apps/mediawiki/linux-ima/index.php?title=Main_Page Given the fact EVM protects IMA data (security.ima), which is generated inline, I am not sure how EVM can sign images offline. I might have misunderstood things, please correct me if that's not the case. Thanks Vivek IMA-appraisal verifies the integrity of file data, while EVM verifies the integrity of the file metadata, such as LSM and IMA-appraisal labels. Both 'security.ima' and 'security.evm' can contain digital signatures. thanks, Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: Kdump with signed images
On Thu, 2012-10-25 at 10:10 -0400, Vivek Goyal wrote: On Thu, Oct 25, 2012 at 02:10:01AM -0400, Mimi Zohar wrote: [..] IMA-appraisal verifies the integrity of file data, while EVM verifies the integrity of the file metadata, such as LSM and IMA-appraisal labels. Both 'security.ima' and 'security.evm' can contain digital signatures. But the private key for creating these digital signature needs to be on the target system? Thanks Vivek Absolutely not. The public key needs to be added to the _ima or _evm keyrings. Roberto Sassu modified dracut and later made equivalent changes to systemd. Both have been upstreamed. Dmitry has a package that labels the filesystem called ima-evm-utils, which supports hash (IMA), hmac(EVM) and digital signatures(both). We're hoping that distro's would label all immutable files, not only elf executables, with digital signatures and mutable files with a hash. thanks, Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC] Kdump with signed images
On Thu, 2012-10-25 at 09:54 -0400, Vivek Goyal wrote: On Thu, Oct 25, 2012 at 01:43:59AM -0400, Mimi Zohar wrote: On Wed, 2012-10-24 at 13:19 -0400, Vivek Goyal wrote: On Tue, Oct 23, 2012 at 09:44:59AM -0700, Eric W. Biederman wrote: Matthew Garrett m...@redhat.com writes: On Tue, Oct 23, 2012 at 10:59:20AM -0400, Vivek Goyal wrote: But what about creation of a new program which can call kexec_load() and execute an unsigned kernel. Doesn't look like that will be prevented using IMA. Like the existing kernel modules, kexec_load() is not file descriptor based. There isn't an LSM or IMA-appraisal hook here. Right. Trusting userspace would require a new system call that passes in a signature of the userspace binary, and the kernel would then have to verify the ELF object in memory in order to ensure that it matches the signature. Verifying that the copy on the filesystem is unmodified isn't adequate - an attacker could simply have paused the process and injected code. I haven't looked at kexec_load() in detail, but like kernel modules, I think the better solution would be to pass a file descriptor, especially if you're discussing a new system call. (cc'ing Kees.) If we decide to go for a new system call, then yes it looks like passing fd will make sense. Verifying the copy on the filesystem at exec time is perfectly adequate for gating extra permissions. Certainly that is the model everywhere else in the signed key chain. Where IMA falls short is there is no offline signing capability in IMA itself. I think EVM may fix that. I'm not sure what you mean by offline signing capability. IMA-appraisal verifies a file's 'security.ima' xattr, which may contain a hash or a digital signature. EVM protects a file's metadata, including 'security.ima'. 'security.evm' can be either an hmac or a digital signature. By offline we mean that signature generation/signing does not happen on system in question. It happens say on build time. IIUC, in case of IMA, security.ima is generated and signed on the system and that means private key needs to be present on the system? The signature for 'security.ima' can definitely be created offline. 'security.evm' can also be created offline, as long as the target's inode and other metadata doesn't change. We wanted something like module signing where modules are signed at build time and verification happens at load time but no private key needs to be present on the system. Modules are identifiable. As long as you have some mechanism to identify mutable vs immutable files during build, there shouldn't be a problem. I'd use the same private key for signing modules and all others. [ CCing lkml. I think it is a good idea to open discussion to wider audience. Also CCing IMA/EVM folks ] thanks! Based on reading following wiki page, looks like EVM also does not allow offline signing capability. And EVM is protecting IMA data to protect against offline attack. If we can assume that unisgned kernels can't be booted on the platform, then EVM might not be a strict requirement in this case. So as you said, one of the main problem with IMA use to verify /sbin/kexec is that IMA does not provide offline signing capability. ? See above. Realistically, the only solution here is for the kernel to verify that the kernel it's about to boot is signed and for it not to take any untrusted executable code from userspace. From an IMA, as opposed to an IMA-appraisal, perspective, kexec is problematic. IMA maintains a measurement list and extends a PCR with the file hash. The measurement list and PCR value are used to attest to the integrity of the running system. As the original measurement list is lost after kexec, but the PCR value hasn't been reset, the measuremnet list and PCR value won't agree. Hogwash. The kernel verifing a signature of /sbin/kexec at exec time is perfectly reasonable, and realistic. In fact finding a way to trust small bits of userspace even if root is compromised seems a far superior model to simply solving the signing problem for /sbin/kexec. Huh? I don't understand what you're suggesting. Once root has been compromised, that's it. Although I do admit some part of the kexec process will need to verify keys on the images we decide to boot. Which keys? Isn't the kernel module key builtin to the kernel and included in the kernel image signature? I think Eric is referring to verifying bzImage signature. One can sign PE/COFF bzImage so IIUC, Eric seems to be suggesting that let /sbin/kexec verify the integrity/authenticity of bzImage and figure a way out to verify integrity/authenticity of /sbin/kexec. Yes that would work. IMA-appraisal/EVM would verify and enforce
Re: Kdump with signed images
On Thu, 2012-10-25 at 14:55 -0400, Vivek Goyal wrote: On Thu, Oct 25, 2012 at 02:40:21PM -0400, Mimi Zohar wrote: On Thu, 2012-10-25 at 10:10 -0400, Vivek Goyal wrote: On Thu, Oct 25, 2012 at 02:10:01AM -0400, Mimi Zohar wrote: [..] IMA-appraisal verifies the integrity of file data, while EVM verifies the integrity of the file metadata, such as LSM and IMA-appraisal labels. Both 'security.ima' and 'security.evm' can contain digital signatures. But the private key for creating these digital signature needs to be on the target system? Thanks Vivek Absolutely not. The public key needs to be added to the _ima or _evm keyrings. Roberto Sassu modified dracut and later made equivalent changes to systemd. Both have been upstreamed. Putting public key in _ima or _evm keyring is not the problem. This is just the verification part. Dmitry has a package that labels the filesystem called ima-evm-utils, which supports hash (IMA), hmac(EVM) and digital signatures(both). We're hoping that distro's would label all immutable files, not only elf executables, with digital signatures and mutable files with a hash. So this labeling (digital signing) can happen at build time? There is nothing inherently preventing it from happening at build time. Elana Reshetova gave a talk at LSS 2012 on modifying RPM http://lwn.net/Articles/518265/. I suspect you need labeling to happen at system install time? If yes, installer does not have the private key to sign anything. The installed system needs to be labeled, but how that occurs is dependent on your environment (eg. flash, rpm based install). Neither of these mechanisms would require the build private key. On a running system, the package installer, after verifying the package integrity, would install each file with the associated 'security.ima' extended attribute. The 'security.evm' digital signature would be installed with an HMAC, calculated using a system unique key. IOW, if distro sign a file, they will most likely put signatures in ELF header (something along the lines of signing PE/COFF binaries). Rusty was definitely against putting the signature in the ELF header for kernel modules. Why would this be any different? But I think you need digital signatures to be put in security.ima which are stored in xattrs and xattrs are not generated till you put file in question on target file system. Thanks Vivek The 'security.ima' digital signature would be created as part of the build process and stored as an extended attribute with the file, like other metadata. On install, the file, extended attributes and other metadata would be copied to the target file system. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC] Kdump with signed images
On Wed, 2012-10-24 at 13:19 -0400, Vivek Goyal wrote: On Tue, Oct 23, 2012 at 09:44:59AM -0700, Eric W. Biederman wrote: Matthew Garrett m...@redhat.com writes: On Tue, Oct 23, 2012 at 10:59:20AM -0400, Vivek Goyal wrote: But what about creation of a new program which can call kexec_load() and execute an unsigned kernel. Doesn't look like that will be prevented using IMA. Like the existing kernel modules, kexec_load() is not file descriptor based. There isn't an LSM or IMA-appraisal hook here. Right. Trusting userspace would require a new system call that passes in a signature of the userspace binary, and the kernel would then have to verify the ELF object in memory in order to ensure that it matches the signature. Verifying that the copy on the filesystem is unmodified isn't adequate - an attacker could simply have paused the process and injected code. I haven't looked at kexec_load() in detail, but like kernel modules, I think the better solution would be to pass a file descriptor, especially if you're discussing a new system call. (cc'ing Kees.) Verifying the copy on the filesystem at exec time is perfectly adequate for gating extra permissions. Certainly that is the model everywhere else in the signed key chain. Where IMA falls short is there is no offline signing capability in IMA itself. I think EVM may fix that. I'm not sure what you mean by offline signing capability. IMA-appraisal verifies a file's 'security.ima' xattr, which may contain a hash or a digital signature. EVM protects a file's metadata, including 'security.ima'. 'security.evm' can be either an hmac or a digital signature. [ CCing lkml. I think it is a good idea to open discussion to wider audience. Also CCing IMA/EVM folks ] thanks! Based on reading following wiki page, looks like EVM also does not allow offline signing capability. And EVM is protecting IMA data to protect against offline attack. If we can assume that unisgned kernels can't be booted on the platform, then EVM might not be a strict requirement in this case. So as you said, one of the main problem with IMA use to verify /sbin/kexec is that IMA does not provide offline signing capability. ? Realistically, the only solution here is for the kernel to verify that the kernel it's about to boot is signed and for it not to take any untrusted executable code from userspace. From an IMA, as opposed to an IMA-appraisal, perspective, kexec is problematic. IMA maintains a measurement list and extends a PCR with the file hash. The measurement list and PCR value are used to attest to the integrity of the running system. As the original measurement list is lost after kexec, but the PCR value hasn't been reset, the measuremnet list and PCR value won't agree. Hogwash. The kernel verifing a signature of /sbin/kexec at exec time is perfectly reasonable, and realistic. In fact finding a way to trust small bits of userspace even if root is compromised seems a far superior model to simply solving the signing problem for /sbin/kexec. Huh? I don't understand what you're suggesting. Once root has been compromised, that's it. Although I do admit some part of the kexec process will need to verify keys on the images we decide to boot. Which keys? Isn't the kernel module key builtin to the kernel and included in the kernel image signature? It should be an option, isn't it? Either /sbin/kexec can try to verify the integrity of kernel or we extend try to extend kexec() system call to also pass the signature of kernel and let kernel verify it (as you mentioned previously). Thanks Vivek As suggested above, please consider passing a file descriptor, at least in addition, if not in lieu of a signature. thanks, Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
