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); -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html