2012-02-01 20:25:21

by Kasatkin, Dmitry

[permalink] [raw]
Subject: [RFC][PATCH v1 0/2] integrity: module integrity verification

Hi,

Here is another module verification patchset, which is based on the recently
upstreamed digital signature support used by EVM and IMA-appraisal.

The initial module verification code was dependent on IMA-appraisal and
was using security.ima extended attribute to store the signature.
Based on some feeback, it was decided to add the ability to store the
signature in a corresponding module signature file <module>.sig and also
make module checking no longer dependent on IMA-appraisal.
Having signature in a separate <module>.sig file allows to use it on filesystem,
which do not support extended attributes, e.g. network file system,
and also copying modules to target system from build system.

The initial module verification code was dependent on IMA-appraisal and
was using security.ima extended attribute to store the signature.
Based on some feeback it was decided to remove IMA dependency and also
add possibility to store the signature in the corresponding signature
file <module>.sig. It allows to use it on filesystems, which do not support
extended attributes and also allows copying of modules from build system to
the target for testing without extended attribute aware tools.

modprobe and insmod have been modified to read signature either from
extended attribute or signature file and pass it as a kernel module
parameter to load_module system call.

Signature generation is done using the same tool as for EVM/IMA: evm-utils.

These 2 patches are available on the top at #next-ima-module branch at
git://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig.git

evm-utils and module-init-tools are available in linux-ima project GIT:
git://linux-ima.git.sourceforge.net / linux-ima/evm-utils
git://linux-ima.git.sourceforge.net / linux-ima/module-init-tools

- Dmitry

Dmitry Kasatkin (2):
integrity: add ima_module_check hook
integrity: verify module integrity based on signature

Documentation/ABI/testing/securityfs-module-check | 17 ++
include/linux/integrity.h | 10 +
kernel/module.c | 20 ++-
security/integrity/Kconfig | 11 +
security/integrity/Makefile | 1 +
security/integrity/module.c | 251 +++++++++++++++++++++
6 files changed, 305 insertions(+), 5 deletions(-)
create mode 100644 Documentation/ABI/testing/securityfs-module-check
create mode 100644 security/integrity/module.c

--
1.7.5.4


2012-02-01 20:25:24

by Kasatkin, Dmitry

[permalink] [raw]
Subject: [RFC][PATCH v1 1/2] integrity: add ima_module_check hook

IMA measures/appraises modules when modprobe or insmod opens and read them.
Unfortunately, there are no guarantees between what is read by userspace and
what is passed to the kernel via load_module system call. This patch adds a
hook called module_check() to verify the integrity of the module being loaded.

Signed-off-by: Dmitry Kasatkin <[email protected]>
---
include/linux/integrity.h | 10 ++++++++++
kernel/module.c | 20 +++++++++++++++-----
2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 66c5fe9..68419d4 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -37,4 +37,14 @@ static inline void integrity_inode_free(struct inode *inode)
return;
}
#endif /* CONFIG_INTEGRITY_H */
+
+#ifdef CONFIG_INTEGRITY_MODULES
+int module_check(const void *hdr, const unsigned long len, char **args);
+#else
+static inline int module_check(const void *buf, unsigned long len, char **args)
+{
+ return 0;
+}
+#endif
+
#endif /* _LINUX_INTEGRITY_H */
diff --git a/kernel/module.c b/kernel/module.c
index 2c93276..9d97928 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -58,6 +58,7 @@
#include <linux/jump_label.h>
#include <linux/pfn.h>
#include <linux/bsearch.h>
+#include <linux/integrity.h>

#define CREATE_TRACE_POINTS
#include <trace/events/module.h>
@@ -2839,6 +2840,7 @@ static struct module *load_module(void __user *umod,
struct load_info info = { NULL, };
struct module *mod;
long err;
+ char *args = NULL;

pr_debug("load_module: umod=%p, len=%lu, uargs=%p\n",
umod, len, uargs);
@@ -2848,6 +2850,16 @@ static struct module *load_module(void __user *umod,
if (err)
return ERR_PTR(err);

+ args = strndup_user(uargs, ~0UL >> 1);
+ if (IS_ERR(args)) {
+ err = PTR_ERR(args);
+ goto free_copy;
+ }
+
+ err = module_check(info.hdr, info.len, &args);
+ if (err < 0)
+ goto free_copy;
+
/* Figure out module layout, and allocate all the memory. */
mod = layout_and_allocate(&info);
if (IS_ERR(mod)) {
@@ -2887,11 +2899,8 @@ static struct module *load_module(void __user *umod,
flush_module_icache(mod);

/* Now copy in args */
- mod->args = strndup_user(uargs, ~0UL >> 1);
- if (IS_ERR(mod->args)) {
- err = PTR_ERR(mod->args);
- goto free_arch_cleanup;
- }
+ mod->args = args;
+ args = NULL;

/* Mark state as coming so strong_try_module_get() ignores us. */
mod->state = MODULE_STATE_COMING;
@@ -2959,6 +2968,7 @@ static struct module *load_module(void __user *umod,
free_module:
module_deallocate(mod, &info);
free_copy:
+ kfree(args);
free_copy(&info);
return ERR_PTR(err);
}
--
1.7.5.4

2012-02-01 20:25:28

by Kasatkin, Dmitry

[permalink] [raw]
Subject: [RFC][PATCH v1 2/2] integrity: verify module integrity based on signature

This patch adds support for verifying module integrity using digital signature.
Module signature is stored in the modules's 'security.ima' extended attribute
or in the file <module>.sig.
Modprobe and insmod pass the signature via init_module system call as the first
module parameter 'ima=' in hexadecimal format. The module hash is calculated and
is verified against the signature.

modprobe and insmod need to know if kernel module signature verification
support is enabled in order to pass 'ima=' module parameter.
securityfs entry is exported to check if digsig verification support is enabled.

Initially module checking is disabled and enabled by writing 1 to
securityfs/module_check.

Signed-off-by: Dmitry Kasatkin <[email protected]>
---
Documentation/ABI/testing/securityfs-module-check | 17 ++
security/integrity/Kconfig | 11 +
security/integrity/Makefile | 1 +
security/integrity/module.c | 251 +++++++++++++++++++++
4 files changed, 280 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/securityfs-module-check
create mode 100644 security/integrity/module.c

diff --git a/Documentation/ABI/testing/securityfs-module-check b/Documentation/ABI/testing/securityfs-module-check
new file mode 100644
index 0000000..e8f7ab5
--- /dev/null
+++ b/Documentation/ABI/testing/securityfs-module-check
@@ -0,0 +1,17 @@
+What: securityfs/module_check
+Date: February 2012
+Contact: Dmitry Kasatkin <[email protected]>
+Description:
+ Integrity provides a hook module_check() which is called from
+ load_module() in order to verify module integrity.
+ Often, when system starts, initramfs is used to perform
+ different initialization tasks before mounting root file system.
+ initramfs is usually verified together with kernel by the
+ boot loader. Public keys to verify modules are not loaded
+ at that point, so to allow initial module loading from initramfs,
+ verification is disabled. After loading keys, module verification
+ can be enabled by:
+
+ echo 1 > <securityfs>/module_check
+
+ After enabling, verification cannot be disabled anymore.
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 5bd1cc1..813c5f6 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,5 +17,16 @@ config INTEGRITY_SIGNATURE
This is useful for evm and module keyrings, when keys are
usually only added from initramfs.

+config INTEGRITY_MODULES
+ bool "Modules integrity checking"
+ depends on SECURITY
+ select INTEGRITY_SIGNATURE
+ default n
+ help
+ This option enables modules integrity checking,
+ using digital signatures
+
+ If unsure, say N.
+
source security/integrity/ima/Kconfig
source security/integrity/evm/Kconfig
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index d43799c..019907c 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -4,6 +4,7 @@

obj-$(CONFIG_INTEGRITY) += integrity.o
obj-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
+obj-$(CONFIG_INTEGRITY_MODULES) += module.o

integrity-y := iint.o

diff --git a/security/integrity/module.c b/security/integrity/module.c
new file mode 100644
index 0000000..ae880eb
--- /dev/null
+++ b/security/integrity/module.c
@@ -0,0 +1,251 @@
+/*
+ * Copyright (C) 2011,2012 Intel Corporation
+ *
+ * Authors:
+ * Dmitry Kasatkin <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: module.c
+ * implements the module hooks: module_check.
+ */
+
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+#include <linux/vmalloc.h>
+#include <linux/crypto.h>
+#include <linux/security.h>
+
+#include "integrity.h"
+
+#define MODULE_DIGEST_SIZE SHA1_DIGEST_SIZE
+
+#define MODULE_CHECK_ENABLE 0x01
+#define MODULE_CHECK_ENABLED 0x02
+#define MODULE_CHECK_ENFORCE 0x04
+
+static int module_check_enable = MODULE_CHECK_ENABLE | MODULE_CHECK_ENFORCE;
+static char *module_hash = "sha1";
+
+static int __init module_check_setup(char *str)
+{
+ if (strncmp(str, "off", 3) == 0)
+ module_check_enable = 0;
+ else if (strncmp(str, "ignore", 3) == 0)
+ module_check_enable &= ~MODULE_CHECK_ENFORCE;
+ return 1;
+}
+__setup("module_check=", module_check_setup);
+
+static int init_desc(struct hash_desc *desc)
+{
+ int rc;
+
+ desc->tfm = crypto_alloc_hash(module_hash, 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(desc->tfm)) {
+ rc = PTR_ERR(desc->tfm);
+ pr_info("MODULE_CHECK: failed to load %s transform: %d\n",
+ module_hash, rc);
+ return rc;
+ }
+ desc->flags = 0;
+ rc = crypto_hash_init(desc);
+ if (rc)
+ crypto_free_hash(desc->tfm);
+ return rc;
+}
+
+/* last page - first page + 1 */
+#define PAGECOUNT(buf, buflen) \
+ ((((unsigned long)(buf + buflen - 1) & PAGE_MASK) >> PAGE_SHIFT) - \
+ (((unsigned long) buf & PAGE_MASK) >> PAGE_SHIFT) + 1)
+
+/* offset of buf in it's first page */
+#define PAGEOFFSET(buf) ((unsigned long)buf & ~PAGE_MASK)
+
+/**
+ * vmalloc_to_sg() - Make scatterlist from vmallocated buffer
+ * @virt: vmallocated buffer
+ * @len: buffer length
+ *
+ * Asynchronous SHA1 calculation is using scatterlist data. This
+ * function can be used to create scatterlist from vmallocated
+ * data buffer.
+ *
+ * Return pointer to scatterlist or NULL.
+ */
+static struct scatterlist *vmalloc_to_sg(const void *virt, unsigned long len)
+{
+ int nr_pages = PAGECOUNT(virt, len);
+ struct scatterlist *sglist;
+ struct page *pg;
+ int i;
+ int pglen;
+ int pgoff;
+
+ sglist = vmalloc(nr_pages * sizeof(*sglist));
+ if (!sglist)
+ return NULL;
+ memset(sglist, 0, nr_pages * sizeof(*sglist));
+ sg_init_table(sglist, nr_pages);
+ for (i = 0; i < nr_pages; i++, virt += pglen, len -= pglen) {
+ pg = vmalloc_to_page(virt);
+ if (!pg)
+ goto err;
+ pgoff = PAGEOFFSET(virt);
+ pglen = min((PAGE_SIZE - pgoff), len);
+ sg_set_page(&sglist[i], pg, pglen, pgoff);
+ }
+ return sglist;
+err:
+ vfree(sglist);
+ return NULL;
+}
+
+static int calc_module_hash(const void *module, const int module_len,
+ char *digest)
+{
+ struct hash_desc desc;
+ struct scatterlist sg[1], *sgp = sg;
+ int rc = -ENOMEM;
+
+ rc = init_desc(&desc);
+ if (rc != 0)
+ return rc;
+
+ sgp = vmalloc_to_sg(module, module_len);
+ if (!sgp)
+ goto err;
+
+ rc = crypto_hash_update(&desc, sgp, module_len);
+ if (!rc)
+ rc = crypto_hash_final(&desc, digest);
+
+ vfree(sgp);
+err:
+ crypto_free_hash(desc.tfm);
+ return rc;
+}
+
+/**
+ * module_check - check module integrity
+ * @hdr: module data
+ * @len: length of the module
+ * @args: module arguments passed to init_module
+ * @return: 0 on success, error code on failure
+ *
+ * Hook to verify module integrity.
+ *
+ */
+int module_check(const void *hdr, const unsigned long len, char **args)
+{
+ u8 digest[MODULE_DIGEST_SIZE];
+ int rc = -EINVAL, siglen;
+ char *mod_args, *sig, *tmp;
+
+ if (!(module_check_enable & MODULE_CHECK_ENABLED))
+ return 0;
+
+ if (strncmp(*args, "ima=", 4) != 0) {
+ pr_err("IMA: missing 'ima=' option\n");
+ goto err1;
+ }
+
+ /* we need to remove module signature from arg list */
+ mod_args = *args + 4;
+ sig = strsep(&mod_args, " \t");
+ tmp = kstrdup(mod_args ?: "", GFP_KERNEL);
+ if (!tmp) {
+ rc = -ENOMEM;
+ goto err1;
+ }
+
+ siglen = strlen(sig)/2;
+ if (hex2bin((u8 *)sig, sig, siglen) < 0)
+ goto err2;
+
+ rc = calc_module_hash(hdr, len, digest);
+ if (rc < 0)
+ goto err2;
+
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_MODULE, sig, siglen,
+ digest, MODULE_DIGEST_SIZE);
+ if (rc) {
+ pr_err("MODULE_CHECK: module verification failed: %d", rc);
+ print_hex_dump(KERN_ERR, "hash: ", DUMP_PREFIX_NONE, 32, 1,
+ digest, MODULE_DIGEST_SIZE, 0);
+ }
+
+err2:
+ kfree(*args);
+ *args = tmp;
+err1:
+ return (module_check_enable & MODULE_CHECK_ENFORCE) ? rc : 0;
+}
+
+#define TMPBUFLEN 12
+
+static ssize_t show_module_check(struct file *filp, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ char tmpbuf[TMPBUFLEN];
+ ssize_t len;
+
+ len = scnprintf(tmpbuf, TMPBUFLEN, "%d\n", module_check_enable);
+ return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
+}
+
+static ssize_t store_module_check(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ int ret, i;
+
+ if (!(module_check_enable & MODULE_CHECK_ENABLE))
+ return 0;
+
+ if (!capable(CAP_SYS_ADMIN) ||
+ (module_check_enable & MODULE_CHECK_ENABLED))
+ return -EPERM;
+
+ ret = kstrtoint_from_user(buf, count, 10, &i);
+ if (ret < 0)
+ return ret;
+
+ if (i != 1)
+ return -EINVAL;
+
+ module_check_enable |= MODULE_CHECK_ENABLED;
+
+ return count;
+}
+
+static const struct file_operations module_ops = {
+ .read = show_module_check,
+ .write = store_module_check,
+};
+
+static struct dentry *module_fs;
+
+static int __init module_check_init(void)
+{
+ module_fs = securityfs_create_file("module_check", S_IRUSR, NULL, NULL,
+ &module_ops);
+ if (IS_ERR(module_fs))
+ return PTR_ERR(module_fs);
+
+ return 0;
+}
+
+static void __exit module_check_cleanup(void)
+{
+ securityfs_remove(module_fs);
+}
+
+module_init(module_check_init);
+module_exit(module_check_cleanup);
+
+MODULE_DESCRIPTION("Module integrity verification");
+MODULE_LICENSE("GPL");
--
1.7.5.4

2012-02-06 01:51:40

by James Morris

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:

> Hi,
>
> Here is another module verification patchset, which is based on the recently
> upstreamed digital signature support used by EVM and IMA-appraisal.

You should cc: Rusty on any changes to the module code.


--
James Morris
<[email protected]>

2012-02-06 06:59:03

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Mon, Feb 6, 2012 at 3:51 AM, James Morris <[email protected]> wrote:
> On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:
>
>> Hi,
>>
>> Here is another module verification patchset, which is based on the recently
>> upstreamed digital signature support used by EVM and IMA-appraisal.
>
> You should cc: Rusty on any changes to the module code.
>

Hello,

Mimi already has pointed that out.
I have sent him an email with the link..

Thanks,
Dmitry

>
> --
> James Morris
> <[email protected]>

2012-02-07 20:54:36

by Rusty Russell

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Mon, 6 Feb 2012 08:59:00 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
> On Mon, Feb 6, 2012 at 3:51 AM, James Morris <[email protected]> wrote:
> > On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:
> >
> >> Hi,
> >>
> >> Here is another module verification patchset, which is based on the recently
> >> upstreamed digital signature support used by EVM and IMA-appraisal.
> >
> > You should cc: Rusty on any changes to the module code.
> >
>
> Hello,
>
> Mimi already has pointed that out.
> I have sent him an email with the link..

Thanks.

Using an external signature (via cmdline arguments) is simple, at
least. Not sure what the userspace side of this looks like?

Cheers,
Rusty.

2012-02-07 21:19:01

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Tue, Feb 7, 2012 at 7:13 PM, Rusty Russell <[email protected]> wrote:
> On Mon, 6 Feb 2012 08:59:00 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
>> On Mon, Feb 6, 2012 at 3:51 AM, James Morris <[email protected]> wrote:
>> > On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:
>> >
>> >> Hi,
>> >>
>> >> Here is another module verification patchset, which is based on the recently
>> >> upstreamed digital signature support used by EVM and IMA-appraisal.
>> >
>> > You should cc: Rusty on any changes to the module code.
>> >
>>
>> Hello,
>>
>> Mimi already has pointed that out.
>> I have sent him an email with the link..
>
> Thanks.
>
> Using an external signature (via cmdline arguments) is simple, at
> least.  Not sure what the userspace side of this looks like?
>

Hello,

There are couple of patches for modprobe and insmod...

You could see them on the top at:
http://linux-ima.git.sourceforge.net/git/gitweb.cgi?p=linux-ima/module-init-tools.git;a=summary

It first tries to read signature from xattr, then from file...
"modprobe -v" will show 'ima=' parameter with signature.

- Dmitry


> Cheers,
> Rusty.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

2012-02-08 00:19:22

by Rusty Russell

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Tue, 7 Feb 2012 23:18:38 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
> On Tue, Feb 7, 2012 at 7:13 PM, Rusty Russell <[email protected]> wrote:
> > On Mon, 6 Feb 2012 08:59:00 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
> >> On Mon, Feb 6, 2012 at 3:51 AM, James Morris <[email protected]> wrote:
> >> > On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:
> >> >
> >> >> Hi,
> >> >>
> >> >> Here is another module verification patchset, which is based on the recently
> >> >> upstreamed digital signature support used by EVM and IMA-appraisal.
> >> >
> >> > You should cc: Rusty on any changes to the module code.
> >> >
> >>
> >> Hello,
> >>
> >> Mimi already has pointed that out.
> >> I have sent him an email with the link..
> >
> > Thanks.
> >
> > Using an external signature (via cmdline arguments) is simple, at
> > least.  Not sure what the userspace side of this looks like?
> >
>
> Hello,
>
> There are couple of patches for modprobe and insmod...
>
> You could see them on the top at:
> http://linux-ima.git.sourceforge.net/git/gitweb.cgi?p=linux-ima/module-init-tools.git;a=summary
>
> It first tries to read signature from xattr, then from file...
> "modprobe -v" will show 'ima=' parameter with signature.
>
> - Dmitry

The problem is that distributions tend to have two variants of modules:
stripped and unstripped. Thus you may want to support multiple
signatures, any *one* of which may match.

I've cc'd the module-init-tools and libkmod maintainers for their
comments, too.

Cheers,
Rusty.

2012-02-08 13:54:31

by Mimi Zohar

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Wed, 2012-02-08 at 10:09 +1030, Rusty Russell wrote:
> On Tue, 7 Feb 2012 23:18:38 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
> > On Tue, Feb 7, 2012 at 7:13 PM, Rusty Russell <[email protected]> wrote:
> > > On Mon, 6 Feb 2012 08:59:00 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
> > >> On Mon, Feb 6, 2012 at 3:51 AM, James Morris <[email protected]> wrote:
> > >> > On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:
> > >> >
> > >> >> Hi,
> > >> >>
> > >> >> Here is another module verification patchset, which is based on the recently
> > >> >> upstreamed digital signature support used by EVM and IMA-appraisal.
> > >> >
> > >> > You should cc: Rusty on any changes to the module code.
> > >> >
> > >>
> > >> Hello,
> > >>
> > >> Mimi already has pointed that out.
> > >> I have sent him an email with the link..
> > >
> > > Thanks.
> > >
> > > Using an external signature (via cmdline arguments) is simple, at
> > > least. Not sure what the userspace side of this looks like?
> > >
> >
> > Hello,
> >
> > There are couple of patches for modprobe and insmod...
> >
> > You could see them on the top at:
> > http://linux-ima.git.sourceforge.net/git/gitweb.cgi?p=linux-ima/module-init-tools.git;a=summary
> >
> > It first tries to read signature from xattr, then from file...
> > "modprobe -v" will show 'ima=' parameter with signature.
> >
> > - Dmitry
>
> The problem is that distributions tend to have two variants of modules:
> stripped and unstripped. Thus you may want to support multiple
> signatures, any *one* of which may match.
>
> I've cc'd the module-init-tools and libkmod maintainers for their
> comments, too.
>
> Cheers,
> Rusty.

Hi Rusty,

As a distro knows what it is shipping, why would you need support for
both stripped/unstripped versions. Unless "stripping" occurs post
install. Perhaps something similar to 'prelink'?

thanks,

Mimi

2012-02-08 14:02:32

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Wed, Feb 8, 2012 at 3:45 PM, Mimi Zohar <[email protected]> wrote:
> On Wed, 2012-02-08 at 10:09 +1030, Rusty Russell wrote:
>> On Tue, 7 Feb 2012 23:18:38 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
>> > On Tue, Feb 7, 2012 at 7:13 PM, Rusty Russell <[email protected]> wrote:
>> > > On Mon, 6 Feb 2012 08:59:00 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
>> > >> On Mon, Feb 6, 2012 at 3:51 AM, James Morris <[email protected]> wrote:
>> > >> > On Wed, 1 Feb 2012, Dmitry Kasatkin wrote:
>> > >> >
>> > >> >> Hi,
>> > >> >>
>> > >> >> Here is another module verification patchset, which is based on the recently
>> > >> >> upstreamed digital signature support used by EVM and IMA-appraisal.
>> > >> >
>> > >> > You should cc: Rusty on any changes to the module code.
>> > >> >
>> > >>
>> > >> Hello,
>> > >>
>> > >> Mimi already has pointed that out.
>> > >> I have sent him an email with the link..
>> > >
>> > > Thanks.
>> > >
>> > > Using an external signature (via cmdline arguments) is simple, at
>> > > least.  Not sure what the userspace side of this looks like?
>> > >
>> >
>> > Hello,
>> >
>> > There are couple of patches for modprobe and insmod...
>> >
>> > You could see them on the top at:
>> > http://linux-ima.git.sourceforge.net/git/gitweb.cgi?p=linux-ima/module-init-tools.git;a=summary
>> >
>> > It first tries to read signature from xattr, then from file...
>> > "modprobe -v" will show 'ima='  parameter with signature.
>> >
>> > - Dmitry
>>
>> The problem is that distributions tend to have two variants of modules:
>> stripped and unstripped.  Thus you may want to support multiple
>> signatures, any *one* of which may match.
>>
>> I've cc'd the module-init-tools and libkmod maintainers for their
>> comments, too.
>>
>> Cheers,
>> Rusty.
>
> Hi Rusty,
>
> As a distro knows what it is shipping, why would you need support for
> both stripped/unstripped versions.  Unless "stripping" occurs post
> install.  Perhaps something similar to 'prelink'?
>

How are they distributed? In separate packages?
And striped during package creation?
Then during package building, before archiving, signing tool is simply
invoked for each binary package,
so "same" modules from different packages will get own signature.

Or it goes some other way?

> thanks,
>
> Mimi
>

2012-02-08 18:03:54

by Rusty Russell

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

On Wed, 8 Feb 2012 16:02:28 +0200, "Kasatkin, Dmitry" <[email protected]> wrote:
> On Wed, Feb 8, 2012 at 3:45 PM, Mimi Zohar <[email protected]> wrote:
> > On Wed, 2012-02-08 at 10:09 +1030, Rusty Russell wrote:
> >> The problem is that distributions tend to have two variants of modules:
> >> stripped and unstripped.  Thus you may want to support multiple
> >> signatures, any *one* of which may match.
> >>
> >> I've cc'd the module-init-tools and libkmod maintainers for their
> >> comments, too.
> > Hi Rusty,
> >
> > As a distro knows what it is shipping, why would you need support for
> > both stripped/unstripped versions.  Unless "stripping" occurs post
> > install.  Perhaps something similar to 'prelink'?
>
> How are they distributed? In separate packages?
> And striped during package creation?
> Then during package building, before archiving, signing tool is simply
> invoked for each binary package,
> so "same" modules from different packages will get own signature.
>
> Or it goes some other way?

I don't know. Perhaps it isn't an issue; David Howells and Jon Masters
might have comments.

Cheers,
Rusty.

2012-02-13 16:21:56

by David Howells

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 0/2] integrity: module integrity verification

Mimi Zohar <[email protected]> wrote:

> As a distro knows what it is shipping, why would you need support for
> both stripped/unstripped versions. Unless "stripping" occurs post
> install. Perhaps something similar to 'prelink'?

Let me summarise the state of things.


In RHEL and Fedora the stripping occurs twice that I know of:

(1) During package generation the debugging info in each module file is
extracted into a separate file in a 'debuginfo' RPM and is then stripped
from the module binary.

(2) During initramfs generation, the module files are further stripped to get
rid of all unnecessary bulk.

I'm told that step (1) can be waived - but that means that the installed module
still has all its debug info in it, and thus uses up more filesystem space.


Red Hat already have a working module signature checker that we've been using
for a few years. However, it incorporates the signature *in* the module file,
using ELF's ability as a container format to carry it in an ELF note.

This has the downside that the kernel then has to determine that the ELF is
sufficiently valid that it's safe to parse it to find the signature note.

The checker is also robust enough that the module survives being stripped a
number of times by tools from a different packages (strip and eu-strip), but
the cost of this is that the kernel has to limit and canonicalise the data it
adds to the signature.

Basically, this checker hasn't required to need any alterations to userspace
tools outside of the kernel sources (eg. busybox, modprobe, insmod, mkinitrd,
strip, kernel rpm specfile, ...) to generate signed modules that can be
checked, and doesn't require any special filesystem features (such as xattrs),
hardware (such as a TPM) or auxiliary files to hold signatures.


However, Rusty doesn't like the RH checker because it is a largish chunk of
code that needs careful checking:

warthog>size module-verify.o module-verify-elf.o module-verify-sig.o
text data bss dec hex filename
170 0 0 170 aa kernel/module-verify.o
2351 0 0 2351 92f kernel/module-verify-elf.o
2390 32 20 2442 98a kernel/module-verify-sig.o

plus a bit in kernel/module.o on x86_64.

He would prefer to see something that just appends the signature with a magic
number and then the kernel just has to search for the magic number and digest
the entirety of the blob up to that point. You don't then need to check the
ELF as the validity of the ELF structure is then guaranteed by the signature.

The main downside of this approach is that the signature is only good as long
as the ELF module is not modified anywhere - and this mean no stripping. If
the unstripped, semi-unstripped and fully stripped modules are all necessary,
then they must all be included in the package, and any variant that is going to
be loaded (which may perhaps only be the fully-stripped variant) must be
signed. Another problem is recognising the signature marker correctly as it's
conceivably possible for the assembler to generate the same string (there are
ways round that, however).


As I understand it, Dmitry's approach is similar to Rusty's except the
signature is determined by userspace (eg. insmod) and passed to the kernel
separately as a special argument to insmod. This allows Dmitry to store the
signature separately, say in an xattr on the module file or in some special
hardware.

The downsides of this approach are similar to Rusty's, and have the additional
problem that insmod & co. may need to implement multiple methods for finding
the signature (are there xattrs in the initramfs, for example?).

However, if there is a real requirement to store the signature detached from
the module (say in a cryptographic hardware cache), then Dmitry's approach may
be the only viable one, and userspace can look through the module file for an
inline signature, and, if found, detach it and give the signature and the
remnant blob to the kernel separately.


At some point I intend to look at integrating Dmitry's stuff with my signature
handling patches to see if we can make a unified system that can handle the
signature being done in software or in hardware, and with any combination of
digest and asymmetric key algorithm. On that basis, if we can't go with RH's
module signature checker, I'd prefer Dmitry's approach over Rusty's and do the
signature retrieval in userspace.

David