All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kyle McMartin <kyle@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: David Howells <dhowells@redhat.com>,
	rusty@rustcorp.com.au, jstancek@redhat.com,
	Stephan Mueller <stephan.mueller@atsec.com>
Subject: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned
Date: Thu, 24 Jan 2013 14:06:10 -0500	[thread overview]
Message-ID: <20130124190610.GI6538@redacted.bos.redhat.com> (raw)
In-Reply-To: <50FFFF48.6020608@atsec.com>

After thinking about it a while, this seems like the best way to solve
the problem, although it does still kind of offend my delicate
sensibilities...

Doing this check in the crypto layer seems kind of like a layering
violation to me (and, to be honest, I think it'd be a gross-hack getting
from the callee back to the caller module.)

Instead, doing it in kernel/module.c and looking at the undefined symbol
list again looks to me like an ordering problem since we're doing the
signature check before we've even done the elf header check. We'd have
to move the panic to a part of the module code that may not necessarily
make sense.

Whereas checking the undefined symbol list at modpost time is fairly
logical, and adding a new MODULE_INFO entry in the CONFIG_CRYPTO_FIPS
case is a well understood thing to do, and should catch all the crypto
registrations regardless of where they exist in-tree...

Seems to build and work with both values of CONFIG_CRYPTO_FIPS.

Thoughts?

Signed-off-by: Kyle McMartin <kyle@redhat.com>

--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2459,8 +2459,10 @@ static int module_sig_check(struct load_info *info)
 		return 0;
 	}
 
-	/* Not having a signature is only an error if we're strict. */
-	if (err < 0 && fips_enabled)
+	/* Not having a signature is only an error if we're strict, and
+	 * the module registers a crypto algorithm (checked in modpost)
+	 */
+	if (err < 0 && fips_enabled && !get_modinfo(info, "crypto_fips"))
 		panic("Module verification failed with error %d in FIPS mode\n",
 		      err);
 	if (err == -ENOKEY && !sig_enforce)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index ff36c50..79f46e2 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1888,6 +1888,23 @@ static void add_staging_flag(struct buffer *b, const char *name)
 		buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n");
 }
 
+static void add_crypto_flag(struct buffer *b, struct module *mod)
+{
+#if defined(CONFIG_CRYPTO_FIPS)
+	struct symbol *s;
+
+	/* iterate unresolved symbols looking for...
+	 *  - crypto_register_algs
+	 */
+	for (s = mod->unres; s; s = s->next) {
+		if (strcmp("crypto_register_algs", s->name) == 0)
+			buf_printf(b, "\nMODULE_INFO(crypto_fips, \"Y\");\n");
+	}
+#else
+	return;
+#endif /*CONFIG_CRYPTO_FIPS*/
+}
+
 /**
  * Record CRCs for unresolved symbols
  **/
@@ -2202,6 +2219,7 @@ int main(int argc, char **argv)
 		add_header(&buf, mod);
 		add_intree_flag(&buf, !external_module);
 		add_staging_flag(&buf, mod->name);
+		add_crypto_flag(&buf, mod);
 		err |= add_versions(&buf, mod);
 		add_depends(&buf, mod, modules);
 		add_moddevtable(&buf, mod);

  parent reply	other threads:[~2013-01-24 19:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-22 18:43 [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set Kyle McMartin
2013-01-22 23:17 ` Rusty Russell
2013-01-23 11:26 ` David Howells
2013-01-23 15:18   ` Stephan Mueller
2013-01-24 14:59     ` Kyle McMartin
2013-01-25 11:28       ` Stephan Mueller
2013-01-24 19:06     ` Kyle McMartin [this message]
2013-01-24 19:21       ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin
2013-01-24 23:36       ` Rusty Russell
2013-01-25  5:45         ` Kyle McMartin
2013-01-25 12:42         ` Stephan Mueller
2013-02-03 23:34           ` Rusty Russell
2013-01-25 12:46         ` Stephan Mueller
2013-01-25 12:18       ` Stephan Mueller
2013-02-05 22:58         ` [RFC PATCH] fips: check whether a module registering an alg or template is signed Kyle McMartin
2013-02-06  8:02           ` Stephan Mueller
2013-02-06 16:15             ` Kyle McMartin
2013-02-06 17:45               ` Stephan Mueller
2013-02-06 18:18                 ` Kyle McMartin
2013-01-25  0:14     ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned David Howells
2013-01-25  3:20       ` Matthew Garrett
2013-01-25 12:23         ` Stephan Mueller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130124190610.GI6538@redacted.bos.redhat.com \
    --to=kyle@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=jstancek@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    --cc=stephan.mueller@atsec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.