Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758828Ab2JaR2U (ORCPT ); Wed, 31 Oct 2012 13:28:20 -0400 Received: from cantor2.suse.de ([195.135.220.15]:33260 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752318Ab2JaR2S (ORCPT ); Wed, 31 Oct 2012 13:28:18 -0400 Date: Wed, 31 Oct 2012 18:28:16 +0100 Message-ID: From: Takashi Iwai To: Matthew Garrett Cc: Jiri Kosina , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-efi@vger.kernel.org Subject: Re: [RFC] Second attempt at kernel secure boot support In-Reply-To: <20121029174131.GC7580@srcf.ucam.org> References: <1348152065-31353-1-git-send-email-mjg@redhat.com> <20121029174131.GC7580@srcf.ucam.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4770 Lines: 148 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 #include +#include #include #include #include @@ -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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/