Re: [RFC] Second attempt at kernel secure boot support

[Takashi Iwai <tiwai@suse.de>]

At Mon, 29 Oct 2012 17:41:31 +0000,
Matthew Garrett wrote:
> 
> On Mon, Oct 29, 2012 at 08:49:41AM +0100, Jiri Kosina wrote:
> > On Thu, 20 Sep 2012, Matthew Garrett wrote:
> > 
> > > This is pretty much identical to the first patchset, but with the capability
> > > renamed (CAP_COMPROMISE_KERNEL) and the kexec patch dropped. If anyone wants
> > > to deploy these then they should disable kexec until support for signed
> > > kexec payloads has been merged.
> > 
> > Apparently your patchset currently doesn't handle device firmware loading, 
> > nor do you seem to mention in in the comments.
> 
> Correct.
> 
> > I believe signed firmware loading should be put on plate as well, right?
> 
> I think that's definitely something that should be covered. I hadn't 
> worried about it immediately as any attack would be limited to machines 
> with a specific piece of hardware, and the attacker would need to expend 
> a significant amount of reverse engineering work on the firmware - and 
> we'd probably benefit from them doing that in the long run...

request_firmware() is used for microcode loading, too, so it's fairly
a core part to cover, I'm afraid.

I played a bit about this yesterday.  The patch below is a proof of
concept to (ab)use the module signing mechanism for firmware loading
too.  Sign firmware files via scripts/sign-file, and put to
/lib/firmware/signed directory.

It's just a rough cut, and the module options are other pieces there
should be polished better, of course.  Also another signature string
should be better for firmware files :)


Takashi

---
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index b34b5cd..2bc8415 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -145,6 +145,12 @@ config EXTRA_FIRMWARE_DIR
 	  this option you can point it elsewhere, such as /lib/firmware/ or
 	  some other directory containing the firmware files.
 
+config FIRMWARE_SIG
+	bool "Firmware signature check"
+	depends on FW_LOADER && MODULE_SIG
+	help
+	  Check the embedded signature of firmware files like signed modules.
+
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"
 	depends on DEBUG_KERNEL
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8945f4e..81fc8a4 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -268,6 +268,12 @@ static void fw_free_buf(struct firmware_buf *buf)
 
 /* direct firmware loading support */
 static const char *fw_path[] = {
+#ifdef CONFIG_FIRMWARE_SIG
+	"/lib/firmware/updates/" UTS_RELEASE "/signed",
+	"/lib/firmware/updates/signed",
+	"/lib/firmware/" UTS_RELEASE "/signed",
+	"/lib/firmware/signed",
+#endif
 	"/lib/firmware/updates/" UTS_RELEASE,
 	"/lib/firmware/updates",
 	"/lib/firmware/" UTS_RELEASE,
@@ -844,6 +850,41 @@ exit:
 	return fw_priv;
 }
 
+#ifdef CONFIG_FIRMWARE_SIG
+/* XXX */
+extern int mod_verify_sig(const void *mod, unsigned long *_modlen);
+
+static bool sig_enforce;
+module_param(sig_enforce, bool, 0444);
+
+static int firmware_sig_check(struct firmware_buf *buf)
+{
+	unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+	long len;
+	int err;
+
+	len = buf->size - markerlen;
+	if (len <= 0 ||
+	    memcmp(buf->data + len, MODULE_SIG_STRING, markerlen)) {
+		pr_debug("%s: no signature found\n", buf->fw_id);
+		return sig_enforce ? -ENOKEY : 0;
+	}
+	err = mod_verify_sig(buf->data, &len);
+	if (err < 0) {
+		pr_debug("%s: signature error: %d\n", buf->fw_id, err);
+		return err;
+	}
+	buf->size = len;
+	pr_debug("%s: signature OK!\n", buf->fw_id);
+	return 0;
+}
+#else
+static inline int firmware_sig_check(struct firmware_buf *buf)
+{
+	return 0;
+}
+#endif
+
 static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
 				  long timeout)
 {
@@ -909,6 +950,9 @@ handle_fw:
 	if (!buf->size || test_bit(FW_STATUS_ABORT, &buf->status))
 		retval = -ENOENT;
 
+	if (!retval)
+		retval = firmware_sig_check(buf);
+
 	/*
 	 * add firmware name into devres list so that we can auto cache
 	 * and uncache firmware for device.
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index ea1b1df..c39f49b 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,6 +11,7 @@
 
 #include <linux/kernel.h>
 #include <linux/err.h>
+#include <linux/export.h>
 #include <crypto/public_key.h>
 #include <crypto/hash.h>
 #include <keys/asymmetric-type.h>
@@ -247,3 +248,4 @@ error_put_key:
 	pr_devel("<==%s() = %d\n", __func__, ret);
 	return ret;	
 }
+EXPORT_SYMBOL_GPL(mod_verify_sig);