Re: [RFC PATCH v2 00/11] vfss: support for a common kernel file loader
On Thu, 2016-01-21 at 21:16 +0100, Luis R. Rodriguez wrote: > On Mon, Jan 18, 2016 at 10:11:15AM -0500, Mimi Zohar wrote: > > > > 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 > > Did 0-day bot get a chance to test this tree? If not can it > be added ? Yes, I get the notifications. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 02/11] vfs: define a generic function to read a file from the kernel
On Wed, 2016-01-20 at 02:09 +0100, Luis R. Rodriguez wrote:
> On Mon, Jan 18, 2016 at 10:11:17AM -0500, Mimi Zohar wrote:
> > diff --git a/fs/exec.c b/fs/exec.c
> > index b06623a..6d623c2 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -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;
>
> loff_t is a __kernel_loff_t, which in turn is a long long, and that's
> signed. We don't catch a negative value here, for max_size, we could
> return -EINVAL if its < 0.
>
> > + if (i_size == 0)
> > + return -EINVAL;
>
> Likewise for i_size. The setter of the size will depend on how the
> code calling this routine setup the struct file passed.
>
> So how about adding a i_size <= 0 check here as well here?
> At least fw_read_file_contents() has historically done this,
> so if this generic read is going to skip that I'd like to
> see why. We're unifying so I rather be more pedantic.
>
> Provided this is addressed feel free to peg:
>
> Reviewed-by: Luis R. Rodriguez
Don't know how I missed that. Thanks!
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 09/11] ima: load policy using path
On Thu, 2016-01-21 at 01:05 +0100, Luis R. Rodriguez wrote:
> On Mon, Jan 18, 2016 at 10:11:24AM -0500, Mimi Zohar wrote:
> > --- 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;
> > +}
> > +
>
> EXPORT_SYMBOL_GPL() needed.
Yes. Thank you for reviewing all the patches!
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect
On Tue, 2016-01-19 at 22:00 +0200, Dmitry Kasatkin wrote: > Hi Mimi, > > Please change > > Signed-off-by: Dmitry Kasatkin I'll make the change here and in the other patches as well. Mimi ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 03/11] ima: provide buffer hash calculation function
On Tue, 2016-01-19 at 21:26 +0200, Dmitry Kasatkin wrote:
> On Mon, Jan 18, 2016 at 5:11 PM, Mimi Zohar wrote:
> > From: Dmitry Kasatkin
> >
> > This patch provides convenient buffer hash calculation function.
> >
> > Changelog:
> > - rewrite to support loff_t sized buffers - Mimi
> > (based on Fenguang Wu's testing)
> >
> > Signed-off-by: Dmitry Kasatkin
> > Signed-off-by: Mimi Zohar
> > ---
> > security/integrity/ima/ima.h| 2 ++
> > security/integrity/ima/ima_crypto.c | 47
> > +
> > 2 files changed, 49 insertions(+)
> >
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index fb8da36..de53631 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry
> > *entry, int violation,
> >const char *op, struct inode *inode,
> >const unsigned char *filename);
> > int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
> > +int ima_calc_buffer_hash(const void *buf, loff_t len,
> > +struct ima_digest_data *hash);
> > int ima_calc_field_array_hash(struct ima_field_data *field_data,
> > struct ima_template_desc *desc, int
> > num_fields,
> > struct ima_digest_data *hash);
> > diff --git a/security/integrity/ima/ima_crypto.c
> > b/security/integrity/ima/ima_crypto.c
> > index fb30ce4..8d86281 100644
> > --- a/security/integrity/ima/ima_crypto.c
> > +++ b/security/integrity/ima/ima_crypto.c
> > @@ -519,6 +519,53 @@ int ima_calc_field_array_hash(struct ima_field_data
> > *field_data,
> > return rc;
> > }
> >
> > +static int calc_buffer_shash_tfm(const void *buf, loff_t size,
> > + struct ima_digest_data *hash,
> > + struct crypto_shash *tfm)
> > +{
> > + SHASH_DESC_ON_STACK(shash, tfm);
> > + unsigned int len;
> > + loff_t offset = 0;
> > + int rc;
> > +
> > + shash->tfm = tfm;
> > + shash->flags = 0;
> > +
> > + hash->length = crypto_shash_digestsize(tfm);
> > +
> > + rc = crypto_shash_init(shash);
> > + if (rc != 0)
> > + return rc;
> > +
> > + len = size < PAGE_SIZE ? size : PAGE_SIZE;
> > + while (offset < size) {
> > + rc = crypto_shash_update(shash, buf + offset, len);
> > + if (rc)
> > + break;
> > + offset += len;
> > + }
> > +
>
> Hello Mimi,
>
> May be this was my earlier patch, but it seems to have a problem of
> accessing beyond end of buffer using the same len.
> When buffer always padded by zeros it is not a problem, but it is a bug.
>
> This seems to be better version.
>
>while (size) {
>len = size < PAGE_SIZE ? size : PAGE_SIZE;
>rc = crypto_shash_update(shash, buf, len);
>if (rc)
>break;
>buf += len;
>size -= len;
> }
>
> Please change my sign-of to: dmitry.kasat...@huawei.com
Good catch! I think I unfortunately introduce this bug. Thank you for
the review!
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version
On Thu, 2016-01-21 at 01:03 +0100, Luis R. Rodriguez wrote:
> On Mon, Jan 18, 2016 at 10:11:23AM -0500, Mimi Zohar wrote:
> > 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.
>
> I think it would help if your cover letter and this patch described
> a bit that some LSMs either prefer to read / check / appraise files
> prior to loading and some other prefer to do that later. You could
> explain the LSM hook preferences and what they do. Then here you
> can explain how this one prefers a hook early, but acknowledge that
> the other one still exists.
Before this patch set, IMA measured/appraised/audited a file before
allowing it to be accessed, causing the file in some cases to be read
twice. This patch set changes that. Files are read into memory and
then measured/appraised/audited.
It's been a while since this hook was added. As I recall, Kees added
the pre module hook to limit loading kernel modules to only those
filesystems that were mounted read-only. I would have to look at each
of the LSMs to see how they're using the hooks.
> So:
>
> kernel_read_file() {
> ...
> security_kernel_read_file();
> ...
> security_kernel_post_read_file();
> ...
> }
>
> > 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
> > @@ -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,
>
> Is the goal to eventually kill the other LSM hooks and just keep the
> file one? If so where is that done in this series? It was not clear.
As mentioned in the cover letter, consolidating the LSM hooks is not
covered in this patch set. I was under the impression that not only
were we defining a common kernel read file function, but that we were
also consolidating the pre and post security hooks as well. By defining
the pre and post security hooks in this patch set, it permits each of
the LSMs to migrate to the new hooks independently of each other. Lets
ask the LSM maintainers what they think.
Paul, Casey, Kees, Jon, Tetsuo does it make sense to consolidate the
module, firmware, and kexec pre and post security hooks and have just
one set of pre and post security kernel_read_file hook instead? Does
it make sense for this patch set to define the new hooks to allow the
LSMs to migrate to it independently of each other?
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version
On Wed, 2016-01-20 at 15:56 -0800, Luis R. Rodriguez wrote:
> On Wed, Jan 20, 2016 at 3:39 PM, Luis R. Rodriguez wrote:
> >> @@ -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, &buf->data, &size, INT_MAX,
> >> + FIRMWARE_CHECK);
> >
> > The way kernel firmware signing was implemented was that we'd first read the
> > foo.sig (or whatever extension we use).
Was there a reason for using a detached signature and not using the same
method as kernel modules?
> The same kernel_read_file() would be
> > used if this gets applied so this would still works well with that. Of
> > course
> > folks using IMA and their own security policy would just disable the kernel
> > fw signing facility.
Right, support for not measuring/appraising the firmware and sig would
be supported in the policy.
> >> 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
> >> };
> >
> > The only thing that is worth questioning here in light of kernel fw signing
> > is
> > what int policy_id do we use? Would we be OK to add FIRMWARE_SIG_CHECK later
> > While at it, perhaps kernel_read_file() last argument should be enum
> > ima_policy_id then. If the policy_id is going to be used elsewhere outside
> > of
> > IMA then perhaps ima.h is not the best place for it ? Would fs.h for type of
> > file be OK ? Then we'd have a list of known file types the kernel reads.
I would definitely prefer the enumeration be defined at the VFS layer.
For example,
enum kernel_read_file_id {
READING_KEXEC_IMAGE,
READING_KEXEC_INITRAMFS,
READING_FIRMWARE,
READING_FIRMWARE_SIG,
READING_MAX_ID
};
Agreed, the last field of kernel_read_file() and the wrappers should be
the enumeration.
> Actually your patch #9 "ima: load policy using path" defines
> kernel_read_file_from_path and since the firmware code uses a path
> this code would be much cleaner if instead you used that. It'd mean
> more code sharing and would make firmware code cleaner. Could you
> re-order that to go first and then later this as its first user?
> Perhaps add the helper for the firmware patch.
Thanks, I missed that. I'll include this change in the next version.
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version
On Tue, 2016-01-19 at 16:10 -0800, Kees Cook wrote: > On Mon, Jan 18, 2016 at 7:11 AM, Mimi Zohar wrote: > > 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 > > Reviewed-by: Kees Cook Thanks! Mimi ___ 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
---
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(&ima_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 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
---
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(&default_appraise_rules[i].list,
&ima_default_rules);
+ if (default_appraise_rules[i].hooks.policy_id == POLICY_CHECK)
+ temp_ima_appraise |= IMA_APPRAISE_POLICY;
}
ima_rules = &ima_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 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
---
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, &pos);
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 4824a4c..f30f564 10
[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
---
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_from_file)(struct
[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
---
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->len < sizeof(*(info->hdr)))
[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 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
---
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, &buf->data, &size, 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 *buf, size_t size);
extern int ima_hash_and_proc
[RFC PATCH v2 09/11] ima: load policy using path
From: Dmitry Kasatkin
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
Signed-off-by: Mimi Zohar
---
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, &pos);
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(&datap, "\n");
+
+ rc = kernel_read_file_from_path(path, &data, &size, 0, POLICY_CHECK);
+ if (rc < 0)
+ return rc;
+
+ datap = data;
+ while (size > 0 && (p = strsep(&datap, "\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(&ima_write_mutex);
if (result < 0)
goto out_free;
- result = ima_parse_add_rule(data);
- mutex_unlock(&ima_write_mutex);
+ if (data[0] == '/')
+ result = ima_read_polic
[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
---
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, &pos);
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.file->f_path, &stat);
- 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 01/11] ima: separate 'security.ima' reading functionality from collect
From: Dmitry Kasatkin
Instead of passing pointers to pointers to ima_collect_measurent() to
read and return the 'security.ima' xattr value, this patch moves the
functionality to the calling process_measurement() to directly read
the xattr and pass only the hash algo to the ima_collect_measurement().
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
---
security/integrity/ima/ima.h | 15 +++
security/integrity/ima/ima_api.c | 15 +++
security/integrity/ima/ima_appraise.c | 25 ++---
security/integrity/ima/ima_crypto.c | 2 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 11 +++
security/integrity/ima/ima_template.c | 2 --
security/integrity/ima/ima_template_lib.c | 1 -
8 files changed, 33 insertions(+), 40 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 585af61..fb8da36 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "../integrity.h"
@@ -140,9 +141,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
int ima_get_action(struct inode *inode, int mask, int function);
int ima_must_measure(struct inode *inode, int mask, int function);
int ima_collect_measurement(struct integrity_iint_cache *iint,
- struct file *file,
- struct evm_ima_xattr_data **xattr_value,
- int *xattr_len);
+ struct file *file, enum hash_algo algo);
void ima_store_measurement(struct integrity_iint_cache *iint, struct file
*file,
const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
@@ -188,8 +187,8 @@ int ima_must_appraise(struct inode *inode, int mask, enum
ima_hooks func);
void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
int func);
-void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len,
- struct ima_digest_data *hash);
+enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
+int xattr_len);
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value);
@@ -221,10 +220,10 @@ static inline enum integrity_status
ima_get_cache_status(struct integrity_iint_c
return INTEGRITY_UNKNOWN;
}
-static inline void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
-int xattr_len,
-struct ima_digest_data *hash)
+static inline enum hash_algo
+ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
{
+ return ima_hash_algo;
}
static inline int ima_read_xattr(struct dentry *dentry,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 1d950fb..e7c7a5d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -18,7 +18,7 @@
#include
#include
#include
-#include
+
#include "ima.h"
/*
@@ -188,9 +188,7 @@ int ima_get_action(struct inode *inode, int mask, int
function)
* Return 0 on success, error code otherwise
*/
int ima_collect_measurement(struct integrity_iint_cache *iint,
- struct file *file,
- struct evm_ima_xattr_data **xattr_value,
- int *xattr_len)
+ struct file *file, enum hash_algo algo)
{
const char *audit_cause = "failed";
struct inode *inode = file_inode(file);
@@ -201,9 +199,6 @@ int ima_collect_measurement(struct integrity_iint_cache
*iint,
char digest[IMA_MAX_DIGEST_SIZE];
} hash;
- if (xattr_value)
- *xattr_len = ima_read_xattr(file->f_path.dentry, xattr_value);
-
if (!(iint->flags & IMA_COLLECTED)) {
u64 i_version = file_inode(file)->i_version;
@@ -213,11 +208,7 @@ int ima_collect_measurement(struct integrity_iint_cache
*iint,
goto out;
}
- /* use default hash algorithm */
- hash.hdr.algo = ima_hash_algo;
-
- if (xattr_value)
- ima_get_hash_algo(*xattr_value, *xattr_len, &hash.hdr);
+ hash.hdr.algo = algo;
result = ima_calc_file_hash(file, &hash.hdr);
if (!result) {
diff --git a/security/integrity/ima/ima_appraise.c
b/security/integrity/ima/ima_appraise.c
index 1873b55..9c2b46b 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
[RFC PATCH v2 04/11] ima: calculate the hash of a buffer using aynchronous hash(ahash)
Setting up ahash has some overhead. Only use ahash to calculate the
hash of a buffer, if the buffer is larger than ima_ahash_minsize.
Signed-off-by: Mimi Zohar
---
security/integrity/ima/ima_crypto.c | 75 -
1 file changed, 73 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c
b/security/integrity/ima/ima_crypto.c
index 8d86281..82c4d3c 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -519,6 +519,63 @@ int ima_calc_field_array_hash(struct ima_field_data
*field_data,
return rc;
}
+static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
+ struct ima_digest_data *hash,
+ struct crypto_ahash *tfm)
+{
+ struct ahash_request *req;
+ struct scatterlist sg;
+ struct ahash_completion res;
+ int rc, ahash_rc = 0;
+
+ hash->length = crypto_ahash_digestsize(tfm);
+
+ req = ahash_request_alloc(tfm, GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ init_completion(&res.completion);
+ ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP,
+ ahash_complete, &res);
+
+ rc = ahash_wait(crypto_ahash_init(req), &res);
+ if (rc)
+ goto out;
+
+ sg_init_one(&sg, buf, len);
+ ahash_request_set_crypt(req, &sg, NULL, len);
+
+ ahash_rc = crypto_ahash_update(req);
+
+ /* wait for the update request to complete */
+ rc = ahash_wait(ahash_rc, &res);
+ if (!rc) {
+ ahash_request_set_crypt(req, NULL, hash->digest, 0);
+ rc = ahash_wait(crypto_ahash_final(req), &res);
+ }
+out:
+ ahash_request_free(req);
+ return rc;
+}
+
+static int calc_buffer_ahash(const void *buf, loff_t len,
+struct ima_digest_data *hash)
+{
+ struct crypto_ahash *tfm;
+ int rc;
+
+ tfm = ima_alloc_atfm(hash->algo);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ rc = calc_buffer_ahash_atfm(buf, len, hash, tfm);
+
+ ima_free_atfm(tfm);
+
+ return rc;
+}
+
static int calc_buffer_shash_tfm(const void *buf, loff_t size,
struct ima_digest_data *hash,
struct crypto_shash *tfm)
@@ -550,8 +607,8 @@ static int calc_buffer_shash_tfm(const void *buf, loff_t
size,
return rc;
}
-int ima_calc_buffer_hash(const void *buf, loff_t len,
-struct ima_digest_data *hash)
+static int calc_buffer_shash(const void *buf, loff_t len,
+struct ima_digest_data *hash)
{
struct crypto_shash *tfm;
int rc;
@@ -566,6 +623,20 @@ int ima_calc_buffer_hash(const void *buf, loff_t len,
return rc;
}
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+struct ima_digest_data *hash)
+{
+ int rc;
+
+ if (ima_ahash_minsize && len >= ima_ahash_minsize) {
+ rc = calc_buffer_ahash(buf, len, hash);
+ if (!rc)
+ return 0;
+ }
+
+ return calc_buffer_shash(buf, len, hash);
+}
+
static void __init ima_pcrread(int idx, u8 *pcr)
{
if (!ima_used_chip)
--
2.1.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH v2 03/11] ima: provide buffer hash calculation function
From: Dmitry Kasatkin
This patch provides convenient buffer hash calculation function.
Changelog:
- rewrite to support loff_t sized buffers - Mimi
(based on Fenguang Wu's testing)
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
---
security/integrity/ima/ima.h| 2 ++
security/integrity/ima/ima_crypto.c | 47 +
2 files changed, 49 insertions(+)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fb8da36..de53631 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry
*entry, int violation,
const char *op, struct inode *inode,
const unsigned char *filename);
int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+struct ima_digest_data *hash);
int ima_calc_field_array_hash(struct ima_field_data *field_data,
struct ima_template_desc *desc, int num_fields,
struct ima_digest_data *hash);
diff --git a/security/integrity/ima/ima_crypto.c
b/security/integrity/ima/ima_crypto.c
index fb30ce4..8d86281 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -519,6 +519,53 @@ int ima_calc_field_array_hash(struct ima_field_data
*field_data,
return rc;
}
+static int calc_buffer_shash_tfm(const void *buf, loff_t size,
+ struct ima_digest_data *hash,
+ struct crypto_shash *tfm)
+{
+ SHASH_DESC_ON_STACK(shash, tfm);
+ unsigned int len;
+ loff_t offset = 0;
+ int rc;
+
+ shash->tfm = tfm;
+ shash->flags = 0;
+
+ hash->length = crypto_shash_digestsize(tfm);
+
+ rc = crypto_shash_init(shash);
+ if (rc != 0)
+ return rc;
+
+ len = size < PAGE_SIZE ? size : PAGE_SIZE;
+ while (offset < size) {
+ rc = crypto_shash_update(shash, buf + offset, len);
+ if (rc)
+ break;
+ offset += len;
+ }
+
+ if (!rc)
+ rc = crypto_shash_final(shash, hash->digest);
+ return rc;
+}
+
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+struct ima_digest_data *hash)
+{
+ struct crypto_shash *tfm;
+ int rc;
+
+ tfm = ima_alloc_tfm(hash->algo);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ rc = calc_buffer_shash_tfm(buf, len, hash, tfm);
+
+ ima_free_tfm(tfm);
+ return rc;
+}
+
static void __init ima_pcrread(int idx, u8 *pcr)
{
if (!ima_used_chip)
--
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 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
> > ---
> > 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 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 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
> > ---
> > 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, &buf->data, &size, 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: [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
[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
---
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, &pos);
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.file->f_path, &stat);
- 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, &image->kernel_buf,
- &image->kernel_buf_len, KEXEC_CHECK);
+ ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
+ &size, 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, &image->initrd_buf,
- &image->initrd_buf_len,
- INITRAMFS_
[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
---
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, &pos);
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_head task_fix
[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
---
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->len < sizeof(*(info->hdr)))
[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
---
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, &buf->data, &size, 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 --git a/include/linux
[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
---
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, &pos);
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(&datap, "\n");
- rc = integrity_read_file(path, &data, POLICY_CHECK);
+ rc = kernel_read_file_from_path(path, &data, &size, 0, POLICY_CHECK);
if (rc < 0)
return rc;
- size = rc;
- datap = data;
+ datap = (char *)data;
while (size > 0 && (p = strsep(&datap, "\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: [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 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 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 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 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 > > --- > > 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] > > > >
Re: [PATCH 00/16] [RFC PATCH] Signed kexec support
On Thu, 2013-09-12 at 09:17 -0700, Greg KH wrote: > On Thu, Sep 12, 2013 at 07:43:36AM -0400, Vivek Goyal wrote: > > On Wed, Sep 11, 2013 at 08:40:23PM -0700, Greg KH wrote: > > > On Tue, Sep 10, 2013 at 05:44:15PM -0400, Vivek Goyal wrote: > > > > Hi, > > > > > > > > Matthew has been posting patches to lock down kernel either due to > > > > secureboot requirements or because of signed modules with signing > > > > enforced. In kernel lock down mode, kexec will be disabled and that > > > > means kdump will not work either. > > > > > > > > These patches sign /sbin/kexec and kernel verifies the signature > > > > and allows loading a kernel if signature verification is successful. > > > > IOW, trust is extended to validly signed user space. > > > > > > > > Currently it works only for statically linked applications. > > > > > > That's a really big restriction, pretty much making it not workable for > > > distros at the moment to use. > > It is a big restriction for general use case of signed user space but in > > this case I am planning to build /sbin/kexec statically and solve the > > kexec/kdump issue. > > > > I have posted kexec-tools patches here. > > > > https://lists.fedoraproject.org/pipermail/kernel/2013-September/004469.html > > > > Once kernel patches get in, I plan to upstream kexec-tools patches too > > and then distros should be able to build /sbin/kexec statically and > > this should work. > > > > Do you forsee a problem with that? > > Your paranoia is admirable in these patches. If they are accepted, that > is a good first step, but what about the other kexec variants out there? > > > > Any chance to change this in the future? > > > > It might but currently I don't have any plans. I see atleast two issues > > with that. > > > > - If we allow dynamic linking for signed binaries, then dynamic libraries > > will have to be signed too. I suspect in that case pretty much whole of > > the user space will have to be signed. I am not sure if distros are > > willing to do that. > > > > - Currently a shared library can be written on disk (unlike executables) > > while it is mapped. That means after signature verification a root just > > has to open and write to shared library and modify code and defeat the > > purpose of signature verfication. > > Then the existing signature verification logic is broken if this is > possible. > > > Probably these issues can be addressed if there is a need. Just that I > > have not looked into it. > > > > > Or just rely on the existing > > > "signed binaries" functionality we have in the kernel today for the > > > kexec binary as well? Wouldn't that be simpler? > > > > Which signed binary mechanism are you referring to? Are you referring to > > using IMA for signature verification? If yes, there are some issues with > > that. > > Yes, IMA. > > > - IMA does not lock down signed binaries in memory. That means after > > signature verification files can potentially be swapped out and be > > attacked there and modified code then can be swapped back in. > > How can you do that? If this is the case, then IMA is pointless and > should be fixed. > > > - IMA caches the signature appraisal resutls and reappraises the things > > based on if file has been modified or not. But this does not detect any > > raw writes to disk. So after signature verification root should be able > > to do some raw writes to disk and IMA will think file signature are > > just fine. > > IMA should be fixed for this problem. > > > - IMA does not have mechanism to keep track of signed applications and > > a mechanism to disallow ptrace() by unsigned applications. That means > > after signature verification root can just ptrace() signed binary and > > modify its code/data. > > Then IMA should be fixed. > > > - IMA provides mechanism for file based signature verificaiton. > > kexec-tools also needs to verify signature of new kernel being loaded. > > Using IMA on bzImage file has same pitfalls where a file can be modified > > after signature verification. > > > > That's why I have extended keyctl() so that signature verification can > > be done on user space buffer. An application can first read a file in > > buffer and then ask kernel to verify signature. And now root should not > > be able to attack it. > > > > So existing IMA does not seem to have been written for an environment > > where all the user space is not signed we don't trust root and root can > > attack a signed binary. And my patches try to fill that gap. > > It sounds like your changes should go into the IMA core code to resolve > the issues there, as I'm sure they want to also protect from the issues > you have pointed out here. Have you talked to those developers about > this? IMA assumes a different threat model and performance tradeoffs. The solutions suggested for the kexec, single userspace application threat model, presumably wouldn't scale very well. Unlike th
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 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 20:37 +, Matthew Garrett wrote: > On Wed, 2013-03-20 at 15:16 -0400, Mimi Zohar wrote: > > On Wed, 2013-03-20 at 18:12 +, Matthew Garrett wrote: > > > 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? > > Right, that'd be the rough idea. Any further runtime policy updates > would presumably need to be signed with a trusted key. I'm really sorry to belabor this point, but can kexec rely on an LSM label to identify a specific file, out of all the files being executed, in a secure boot environment? The SELinux integrity rule for kexec would then look something like, appraise func=BPRM_CHECK obj_type=kdump_exec_t appraise_type=imasig We could then follow this up with Serge's idea of, "a capset akin to the bounding set, saying you can only have the caps in this set if the running binary was a signed one." kexec already requires CAP_SYS_BOOT. thanks, 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: [PATCH 01/12] Security: Add CAP_COMPROMISE_KERNEL
On Wed, 2013-03-20 at 16:49 +, Matthew Garrett wrote: > On Wed, 2013-03-20 at 12:41 -0400, Mimi Zohar wrote: > > > Matthrew, perhaps you could clarify whether this will be tied to MAC > > security. Based on the kexec thread, I'm under the impression that is > > not the intention, or at least not for kexec. As root isn't trusted, > > neither is the boot command line, nor any policy that is loaded by root, > > including those for MAC. > > The work done on signed initramfs fragments would seem to be the best > option here so far? 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. thanks, 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 Tue, 2013-03-19 at 15:47 +1100, James Morris wrote: > On Mon, 18 Mar 2013, Matthew Garrett wrote: > > > This patch introduces CAP_COMPROMISE_KERNEL. > > I'd like to see this named CAP_MODIFY_KERNEL, which is more accurate and > less emotive. Otherwise I think core kernel developers will be scratching > their head over where to sprinkle this. > > Apart from that, I like the idea, especially when it's wired up to MAC > security. Matthrew, perhaps you could clarify whether this will be tied to MAC security. Based on the kexec thread, I'm under the impression that is not the intention, or at least not for kexec. As root isn't trusted, neither is the boot command line, nor any policy that is loaded by root, including those for MAC. 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 writes: > > > On Thu, Nov 08, 2012 at 01:03:17PM -0800, Eric W. Biederman wrote: > >> Vivek Goyal 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 Thu, 2012-11-01 at 09:53 -0400, Vivek Goyal wrote: > On Thu, Nov 01, 2012 at 09:10:03AM -0400, Vivek Goyal wrote: > > [..] > > > > > > > - So say we can sign /sbin/kexec at build time and distros can do that. > > > > - Verify the signature at exec time using kernel keyring and if > > > > verification happens successfully, say process gains extra capability. > > > > - Use this new capability to determine whether kexec_load() will be > > > > successful or not. > > > > > > > > Even if we can do all this, it still has the issue of being able to > > > > stop the process in user space and replace the code at run time > > > > and be able to launch unsigned kernel. > > > > Thinking more about it. Can we just keep track whether a process was > > ptraced or not and disallow kexec_load() syscall if it was ptraced. > > (I am assuming that ptrace is the only way to change process code/data). > > > > So binaries can be signed offline. Signature verification can take place > > using kernel keyring at exec() time. And we can keep track of ptraced > > processes and disallow calling kexec_load() for such processes. If this > > is implementable, this should take care of following requirement raised > > by matthew. > > > > > > 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, and it > > must be impossible for anything other than /sbin/kexec to make the kexec > > system call. > > * > > > > Thoughts? > > Eric responded but my mistake he responded to only me. So I will quickly > put his idea here. > > [start quote] > > You can't ptrace a process that has a capability you don't. > > That should be enforced in security/commoncap/ > > [end quote] > > This looks like a good idea. Upon verification signed binaries will be > assigned special capability and then no unsigned binary should be able > to ptrace signed/verified processes That's a good generic solution, which I'm all in favor of, but it doesn't resolve the latter half of Matthrew's requirement "and it must be impossible for anything other than /sbin/kexec to make the kexec system call." 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 13:06 -0400, Vivek Goyal wrote: > On Fri, Oct 26, 2012 at 03:39:16AM +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. 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, and it must be impossible for anything other than > > /sbin/kexec to make the kexec system call. > > I am kind of lost now so just trying to summarize whatever I have > learned so far from this thread. Thanks for summarizing. > - So say we can sign /sbin/kexec at build time and distros can do that. > - Verify the signature at exec time using kernel keyring and if > verification happens successfully, say process gains extra capability. > - Use this new capability to determine whether kexec_load() will be > successful or not. > > Even if we can do all this, it still has the issue of being able to > stop the process in user space and replace the code at run time > and be able to launch unsigned kernel. > > So until and unless we have a good solution to verify application's > integrity/authneticity at the time of kexec_load() system call we > still have the problem. And I don't think we have come up with a > solution for that yet (until and unless I missed something). > > Thanks > Vivek > Agreed, you need a new LSM/integrity hook. 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 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 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 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 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
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 Wed, 2012-10-24 at 23:44 -0700, Kees Cook wrote: > On Wed, Oct 24, 2012 at 10:43 PM, 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 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.) > > Yeah, it looks like kexec_load could use a nearly identical new > syscall that uses an fd, just like init_module is getting. > > Another area, kind of related, is firmware loading. The interface for > that is a bit weird, if the documentation is up to date: > > echo 1 > /sys/$DEVPATH/loading > cat $HOTPLUG_FW_DIR/$FIRMWARE > /sysfs/$DEVPATH/data > echo 0 > /sys/$DEVPATH/loading > > It looks like there's a filp on the reader: > > static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj, > struct bin_attribute *bin_attr, > char *buffer, loff_t offset, size_t count) > > But it's not clear to me yet if we'll actually get the firmware file, > or if we'll get a random pipe we can't evaluate. Has anyone looked at > handling "signed" firmware loading yet? > > -Kees Only looked at it enough to mention at LSS, that it's needed. 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 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: [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 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
