Re: [PATCH 21/21] MODSIGN: Apply signature checking to modules on module load [ver #3]

[Rusty Russell]

On Mon, 12 Dec 2011 01:21:40 +0000, David Howells <dhowells@xxxxxxxxxx> wrote:
> Rusty Russell <rusty@xxxxxxxxxx> wrote:
> 
> > I think you misunderstand, I'm talking about the modinfo command, not
> > the .modinfo section.
> 
> Sorry, yes.  But why do you need to enhance modinfo?

I was suggesting that you want it to print the signatures, or at least
indicate their existence.  Maybe check them too, but that might be a bit
too heavy for modinfo.

> > But I need to know exactly what these version-dependent mangling of
> > modules is.  Is it real?  Is it more than strip?  Is it so hard to fix
> > that it makes sense to add 450 lines of dense kernel code to allow
> > alteration of a module after signing?
> 
> The strip program (as far as I know that's the only binutil that we need worry
> about) rearranges and reorders the section, symbol and relocation tables when
> it discards stuff, and different versions of strip have done it
> differently.

OK, then you need to generate stripped modules as part of the build,
too.  It's a bit of a pain, sure, but hardly a showstopper.

> However, you said it should be fairly easy to jump over the ELF parcel to get
> to the signature.  How do you plan on doing that?

> I presume you would just parse sufficient of the ELF to find the
> theoretical ELF EOF and then look there for a whole string of
> signatures

You could do that.  But there's an easier way.  Took me longer to figure
out the damn crypto API than actually write the module part :(

Subject: module: simple signature support.

A signature contains a magic marker: it signs everything up to the
magic marker (ie. just append them):
	SUM=`md5sum drivers/block/loop.ko | cut -d\  -f1`; echo "@Module signature:$SUM" >> drivers/block/loop.ko

We can have false positives, but at worst that make us report EINVAL
(bad signature) instead of ENOENT (no signature).

diff --git a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2374,6 +2374,98 @@ free_hdr:
 	return err;
 }
 
+/* CONFIG_MODULE_SIGN implies we don't trust modules: verify signature
+ * before we interpret (almost) anything. */
+#define MOD_SIGNATURE "@Module signature:"
+
+#include <linux/ctype.h>
+#include <crypto/hash.h>
+#include <crypto/md5.h>
+
+static int from_hex(char c)
+{
+	if (isdigit(c))
+		return c - '0';
+	if (isupper(c))
+		return c - 'A' + 10;
+	return c - 'a' + 10;
+}
+
+/* A signature signs everything before it. */
+static int try_signature(void *data, void *sig, unsigned long max_sig)
+{
+	unsigned long data_len = sig - data;
+
+	sig += strlen(MOD_SIGNATURE);
+	max_sig -= strlen(MOD_SIGNATURE);
+
+	/* Dummy: accept md5 as signature. */
+	{
+		struct crypto_api_blows {
+			struct shash_desc md5;
+			char morestuff[100];
+		} m;
+		u8 digest[MD5_DIGEST_SIZE], expected[MD5_DIGEST_SIZE];
+		char *s = sig;
+		int i;
+
+		/* Not a signature? */
+		if (max_sig < MD5_DIGEST_SIZE * 2) {
+			printk("Too close to end (%lu)\n", max_sig);
+			return -ENOENT;
+		}
+
+		for (i = 0; i < MD5_DIGEST_SIZE * 2; i += 2) {
+			/* Not a signature? */
+			if (!isxdigit(s[i]) || !isxdigit(s[i+1])) {
+				printk("Not hex digit (%c)\n", s[i]);
+				return -ENOENT;
+			}
+			digest[i/2] = (from_hex(s[i])<<4) | from_hex(s[i+1]);
+		}
+
+		m.md5.tfm = crypto_alloc_shash("md5", 0, 0);
+		if (IS_ERR(m.md5.tfm))
+			return PTR_ERR(m.md5.tfm);
+		m.md5.flags = 0;
+
+		crypto_shash_digest(&m.md5, data, data_len, expected);
+		crypto_free_shash(m.md5.tfm);
+		
+		if (memcmp(digest, expected, sizeof(digest)) != 0) {
+			printk("Mismatch: given %02x%02x%02x...,"
+			       " expect %02x%02x%02x...\n",
+			       digest[0], digest[1], digest[2],
+			       expected[0], expected[1], expected[2]);
+			return -EINVAL;
+		}
+		printk("Found valid signature!\n");
+		return 0;
+	}
+}
+
+/* -ENOENT if no signature found, -EINVAL if invalid, 0 if good. */
+static int find_and_check_signatures(struct load_info *info)
+{
+	void *p = info->hdr, *end = p + info->len;
+	const size_t sigsize = strlen(MOD_SIGNATURE);
+	int err = -ENOENT;
+
+	/* Poor man's memmem. len > sigsize */
+	while ((p = memchr(p, MOD_SIGNATURE[0], end - p))) {
+		if (p + sigsize > end)
+			break;
+
+		if (memcmp(p, MOD_SIGNATURE, sigsize) == 0) {
+			err = try_signature(info->hdr, p, end - p);
+			if (!err)
+				break;
+		}
+		p++;
+	}
+	return err;
+}
+
 static void free_copy(struct load_info *info)
 {
 	vfree(info->hdr);
@@ -2819,6 +2911,11 @@ static struct module *load_module(void _
 	if (err)
 		return ERR_PTR(err);
 
+	/* Before we trust it, carefully check signatures. */
+	err = find_and_check_signatures(&info);
+	if (err)
+		goto free_copy;
+
 	/* Figure out module layout, and allocate all the memory. */
 	mod = layout_and_allocate(&info);
 	if (IS_ERR(mod)) {


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/