| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 8 Jun 2015 12:56:44 -0700
From: Kees Cook <keescook@...omium.org>
To: "Luis R. Rodriguez" <mcgrof@...not-panic.com>
Cc: Ming Lei <ming.lei@...onical.com>,
Rusty Russell <rusty@...tcorp.com.au>,
Linus Torvalds <torvalds@...ux-foundation.org>,
David Howells <dhowells@...hat.com>,
seth.forshee@...onical.com, LKML <linux-kernel@...r.kernel.org>,
Paul Bolle <pebolle@...cali.nl>,
linux-wireless <linux-wireless@...r.kernel.org>,
Greg KH <gregkh@...uxfoundation.org>, jlee@...e.com,
Takashi Iwai <tiwai@...e.de>,
Casey Schaufler <casey@...aufler-ca.com>,
Matthew Garrett <mjg59@...f.ucam.org>,
Andrew Morton <akpm@...ux-foundation.org>,
"Luis R. Rodriguez" <mcgrof@...e.com>,
Kyle McMartin <kyle@...nel.org>,
David Woodhouse <David.Woodhouse@...el.com>
Subject: Re: [RFC v3 2/2] firmware: add firmware signature checking support
On Mon, May 18, 2015 at 5:45 PM, Luis R. Rodriguez
<mcgrof@...not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@...e.com>
>
> Systems that have module signing currently enabled may
> wish to extend vetting of firmware passed to the kernel
> as well. We can re-use most of the code for module signing
> for firmware signature verification and signing. This will
> also later enable re-use of this same code for subsystems
> that wish to provide their own cryptographic verification
> requirements on userspace data needed.
>
> Contrary to module signing the signature files are expected
> to be completely detached for practical and licensing puposes.
> If you have foo.bin, you'll need foo.bin.p7s file present
> for firmware signing.
>
> There's both a config option and a boot parameter which control
> whether we accept or fail with unsigned firmware and firmware
> that are signed with an unknown key. If firmware signing is
> enabled permissively we'll only log into the kernel ring buffer
> when use of an unsigned firmware file is used. If firmware
> signing is in enforced mode the kernel will not allow invalid
> or unsigned firmware files to be loaded into the kernel.
>
> Contrary to module signing we do not taint the kernel in
> the permissive fw signing mode due to restrictions on the
> firmware_class API, extensions to enable this are expected
> however in the future.
>
> Cc: Rusty Russell <rusty@...tcorp.com.au>
> Cc: David Howells <dhowells@...hat.com>
> Cc: Ming Lei <ming.lei@...onical.com>
> Cc: Seth Forshee <seth.forshee@...onical.com>
> Cc: Kyle McMartin <kyle@...nel.org>
> Cc: David Woodhouse <David.Woodhouse@...el.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@...e.com>
> ---
> Documentation/firmware_class/signing.txt | 90 +++++++++++++
> drivers/base/Kconfig | 18 +++
> drivers/base/firmware_class.c | 213 ++++++++++++++++++++++++++++++-
> 3 files changed, 314 insertions(+), 7 deletions(-)
> create mode 100644 Documentation/firmware_class/signing.txt
>
> diff --git a/Documentation/firmware_class/signing.txt b/Documentation/firmware_class/signing.txt
> new file mode 100644
> index 0000000..b03f654
> --- /dev/null
> +++ b/Documentation/firmware_class/signing.txt
> @@ -0,0 +1,90 @@
> + ================================
> + KERNEL FIRMWARE SIGNING FACILITY
> + ================================
> +
> +CONTENTS
> +
> + - Overview.
> + - Configuring firmware signing.
> + - Using signing keys.
> + - Signing firmware files.
> +
> +
> +========
> +OVERVIEW
> +========
> +
> +Device drivers which require a firmware to be uploaded onto a device as its own
> +device's microcode use any of the following APIs:
> +
> + * request_firmware()
> + * request_firmware_direct()
> + * request_firmware_nowait()
> +
> +The kernel firmware signing facility enables to cryptographically sign
> +firmware files on a system using the same keys used for module signing.
> +Firmware files's signatures consist of PKCS#7 messages of the respective
> +firmware file. A firmware file named foo.bin, would have its respective
> +signature on the filesystem as foo.bin.p7s. When firmware signature
> +checking is enabled (FIRMWARE_SIG) and when one of the above APIs is used
> +against foo.bin, the file foo.bin.p7s will also be looked for. If
> +FIRMWARE_SIG_FORCE is enabled the foo.bin file will only be allowed to
> +be returned to callers of the above APIs if and only if the foo.bin.p7s
> +file is confirmed to be a valid signature of the foo.bin file. If
> +FIRMWARE_SIG_FORCE is not enabled and only FIRMWARE_SIG is enabled the
> +kernel will be permissive and enabled unsigned firmware files, or firmware
> +files with incorrect signatures. If FIRMWARE_SIG is not enabled the
> +signature file is ignored completely.
> +
> +Firmware signing increases security by making it harder to load a malicious
> +firmware into the kernel. The firmware signature checking is done by the
> +kernel so that it is not necessary to have trusted userspace bits.
> +
> +============================
> +CONFIGURING FIRMWARE SIGNING
> +============================
> +
> +The firmware signing facility is enabled by going to the section:
> +
> +-> Device Drivers
> + -> Generic Driver Options
> + -> Userspace firmware loading support (FW_LOADER [=y])
> + -> Firmware signature verification (FIRMWARE_SIG [=y])
> +
> +If you want to not allow unsigned firmware to be loaded you should
> +enable:
> +
> +"Require all firmware to be validly signed" (FIRMWARE_SIG_FORCE [=y]),
> +under the same menu.
> +
> +==================
> +USING SIGNING KEYS
> +==================
> +
> +The same key types used for module signing can be used for firmware
> +signing. For details on that refer to Documentation/module-signing.txt.
> +
> +You will need:
> +
> + A) A DER-encoded X.509 certificate containing the public key.
> + B) A DER-encoded PKCS#7 message containing the signatures, these are
> + the .p7s files.
> + C) A binary blob that is the detached data for the PKCS#7 message, this
> + is the firmware files
> +
> +A) is must be made available to the kernel. One way to do this is to provide a
> +DER-encoded in the source directory as <name>.x509 when you build the kernel.
> +
> +======================
> +SIGNING FIRMWARE FILES
> +======================
> +
> +To generate a DER-encoded PKCS#7 signature message for each firmware file
> +you can use openssl as follows:
> +
> + openssl smime -sign -in $FIRMWARE_BLOB_NAME \
> + -outform DER \
> + -inkey $PRIVATE_KEY_FILE_IN_PEM_FORM \
> + -signer $X509_CERT_FILE_IN_PEM_FORM \
> + -nocerts -md $DIGEST_ALGORITHM -binary > \
> + $(FIRMWARE_BLOB_NAME).p7s
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 98504ec..fa076ea 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -85,6 +85,24 @@ config FW_LOADER
> require userspace firmware loading support, but a module built
> out-of-tree does.
>
> +config FIRMWARE_SIG
> + bool "Firmware signature verification"
> + depends on FW_LOADER
> + select SYSDATA_SIG
> + help
> + Check firmware files for valid signatures upon load: if the firmware
> + was called foo.bin, a respective foo.bin.p7s is expected to be
> + present as the signature. For more information see
> + Documentation/firmware_class/signing.txt
> +
> +config FIRMWARE_SIG_FORCE
> + bool "Require all firmware to be validly signed"
> + depends on FIRMWARE_SIG
> + help
> + Reject unsigned files or signed files for which we don't have a
> + key. Without this, you'll only get a record on the kernel ring
> + buffer of firmware files loaded without a signature.
> +
> config FIRMWARE_IN_KERNEL
> bool "Include in-kernel firmware blobs in kernel binary"
> depends on FW_LOADER
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 134dd77..97cab65 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -29,6 +29,7 @@
> #include <linux/syscore_ops.h>
> #include <linux/reboot.h>
> #include <linux/security.h>
> +#include <keys/system_keyring.h>
>
> #include <generated/utsrelease.h>
>
> @@ -38,6 +39,11 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
> MODULE_DESCRIPTION("Multi purpose firmware loading support");
> MODULE_LICENSE("GPL");
>
> +static bool fw_sig_enforce = IS_ENABLED(CONFIG_FIRMWARE_SIG_FORCE);
> +#ifndef CONFIG_FIRMWARE_SIG_FORCE
> +module_param(fw_sig_enforce, bool_enable_only, 0644);
> +#endif /* !CONFIG_FIRMWARE_SIG_FORCE */
> +
> /* Builtin firmware support */
>
> #ifdef CONFIG_FW_LOADER
> @@ -142,6 +148,9 @@ struct firmware_buf {
> unsigned long status;
> void *data;
> size_t size;
> + void *data_sig;
> + size_t size_sig;
> + bool sig_ok;
> #ifdef CONFIG_FW_LOADER_USER_HELPER
> bool is_paged_buf;
> bool need_uevent;
> @@ -151,6 +160,7 @@ struct firmware_buf {
> struct list_head pending_list;
> #endif
> const char *fw_id;
> + const char *fw_sig;
> };
>
> struct fw_cache_entry {
> @@ -180,17 +190,33 @@ static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
> struct firmware_cache *fwc)
> {
> struct firmware_buf *buf;
> + const char *sign_ext = ".p7s";
> + char *signed_name;
> +
> + signed_name = kzalloc(PATH_MAX, GFP_ATOMIC);
> + if (!signed_name)
> + return NULL;
>
> buf = kzalloc(sizeof(*buf), GFP_ATOMIC);
> - if (!buf)
> + if (!buf) {
> + kfree(signed_name);
> return NULL;
> + }
>
> buf->fw_id = kstrdup_const(fw_name, GFP_ATOMIC);
> if (!buf->fw_id) {
> + kfree(signed_name);
> kfree(buf);
> return NULL;
> }
>
> + strcpy(signed_name, buf->fw_id);
> + strncat(signed_name, sign_ext, strlen(sign_ext));
fw_id is potentially unbounded, so using strncat hear poses an
overflow risk. Maybe better to use strlcpy?
> + buf->fw_sig = kstrdup_const(signed_name, GFP_ATOMIC);
> + if (!buf->fw_sig)
> + goto out;
> +
> +
> kref_init(&buf->ref);
> buf->fwc = fwc;
> init_completion(&buf->completion);
> @@ -201,6 +227,11 @@ static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
> pr_debug("%s: fw-%s buf=%p\n", __func__, fw_name, buf);
>
> return buf;
> +out:
> + kfree(signed_name);
> + kfree_const(buf->fw_id);
> + kfree(buf);
> + return NULL;
> }
>
> static struct firmware_buf *__fw_lookup_buf(const char *fw_name)
> @@ -262,6 +293,7 @@ static void __fw_free_buf(struct kref *ref)
> #endif
> vfree(buf->data);
> kfree_const(buf->fw_id);
> + kfree_const(buf->fw_sig);
> kfree(buf);
> }
>
> @@ -325,7 +357,84 @@ fail:
> return rc;
> }
>
> +#ifdef CONFIG_FIRMWARE_SIG_FORCE
> +static struct file *get_filesystem_file_sig(const char *sig_name)
> +{
> + return filp_open(sig_name, O_RDONLY, 0);
> +}
> +
> +static bool get_filesystem_file_sig_ok(struct file *file_sig)
> +{
> + if (IS_ERR(file_sig))
> + return -EINVAL;
> + return 0;
> +}
> +
> +static int read_file_signature_contents(struct file *file_sig,
> + struct firmware_buf *fw_buf)
> +{
> + int rc;
> +
> + rc = __read_file_contents(file_sig,
> + &fw_buf->data_sig,
> + &fw_buf->size_sig);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +#elif CONFIG_FIRMWARE_SIG
> +static struct file *get_filesystem_file_sig(const char *sig_name)
> +{
> + struct file *file;
> +
> + file = filp_open(sig_name, O_RDONLY, 0);
> + if (IS_ERR(file))
> + pr_info("singature %s not present, but this is OK\n", sig_name);
> +
> + return file;
> +}
> +
> +static bool get_filesystem_file_sig_ok(struct file *file_sig)
> +{
> + return 0;
> +}
> +
> +static int read_file_signature_contents(struct file *file_sig,
> + struct firmware_buf *fw_buf)
> +{
> + int rc;
> +
> + rc = __read_file_contents(file,
> + &fw_buf->data_sig,
> + &fw_buf->size_sig);
> + if (rc)
> + pr_info("could not read signature %s, but this is OK\n",
> + fw_buf->fw_sig);
> +
> + return 0;
> +}
> +#else
> +static struct file *get_filesystem_file_sig(const char *sig_name)
> +{
> + return NULL;
> +}
> +
> +static bool get_filesystem_file_sig_ok(struct file *file_sig)
> +{
> + return 0;
> +}
> +
> +static int read_file_signature_contents(struct file *file_sig,
> + struct firmware_buf *fw_buf)
> +{
> + return 0;
> +}
> +#endif
> +
> static int fw_read_file_contents(struct file *file,
> + struct file *file_sig,
> struct firmware_buf *fw_buf)
> {
> int rc;
> @@ -336,6 +445,10 @@ static int fw_read_file_contents(struct file *file,
> if (rc)
> return rc;
>
> + rc = read_file_signature_contents(file_sig, fw_buf);
> + if (rc)
> + return rc;
> +
> return 0;
> }
>
> @@ -343,15 +456,20 @@ static int fw_get_filesystem_firmware(struct device *device,
> struct firmware_buf *buf)
> {
> int i, len;
> - int rc = -ENOENT;
> - char *path;
> + int rc = -ENOMEM;
> + char *path, *path_sig = NULL;
>
> path = __getname();
> if (!path)
> return -ENOMEM;
>
> + path_sig = __getname();
> + if (!path_sig)
> + goto out;
> +
> for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
> struct file *file;
> + struct file *file_sig;
>
> /* skip the unset customized path */
> if (!fw_path[i][0])
> @@ -364,18 +482,43 @@ static int fw_get_filesystem_firmware(struct device *device,
> break;
> }
>
> + len = snprintf(path_sig, PATH_MAX, "%s/%s",
> + fw_path[i], buf->fw_sig);
> + if (len >= PATH_MAX) {
> + rc = -ENAMETOOLONG;
> + break;
> + }
> +
> file = filp_open(path, O_RDONLY, 0);
> - if (IS_ERR(file))
> + if (IS_ERR(file)) {
> + rc = -ENOENT;
> continue;
> - rc = fw_read_file_contents(file, buf);
> + }
> +
> + file_sig = get_filesystem_file_sig(path_sig);
> + rc = get_filesystem_file_sig_ok(file_sig);
> + if (rc) {
> + fput(file);
> + if (!IS_ERR(file_sig))
> + fput(file_sig);
> + continue;
> + }
> +
> + rc = fw_read_file_contents(file, file_sig, buf);
> +
> fput(file);
> + if (!IS_ERR(file_sig))
> + fput(file_sig);
> +
> if (rc)
> dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n",
> path, rc);
> else
> break;
> }
> +out:
> __putname(path);
> + __putname(path_sig);
>
> if (!rc) {
> dev_dbg(device, "firmware: direct-loading firmware %s\n",
> @@ -410,11 +553,42 @@ static void fw_set_page_data(struct firmware_buf *buf, struct firmware *fw)
> fw->size = buf->size;
> fw->data = buf->data;
>
> - pr_debug("%s: fw-%s buf=%p data=%p size=%u\n",
> + pr_debug("%s: fw-%s buf=%p data=%p size=%u sig_ok=%d\n",
> __func__, buf->fw_id, buf, buf->data,
> - (unsigned int)buf->size);
> + (unsigned int)buf->size, buf->sig_ok);
> }
>
> +#ifdef CONFIG_FIRMWARE_SIG
> +static int firmware_sig_check(struct firmware *fw, const char *name)
> +{
> + int err = -ENOKEY;
> + struct firmware_buf *buf = fw->priv;
> + const void *data = buf->data;
> + const void *data_sig = buf->data_sig;
> +
> + err = system_verify_data(data, buf->size, data_sig, buf->size_sig);
> + if (!err) {
> + buf->sig_ok = true;
> + fw_set_page_data(buf, fw);
> + return 0;
> + }
> +
> + /* Not having a signature is only an error if we're strict. */
> + if (err == -ENOKEY && !fw_sig_enforce)
> + err = 0;
> +
> + fw_set_page_data(buf, fw);
> +
> + return err;
> +}
> +#else /* !CONFIG_FIRMWARE_SIG */
> +static int firmware_sig_check(struct firmware *fw, const char *name)
> +{
> + return 0;
> +}
> +#endif /* !CONFIG_MODULE_SIG */
> +
> +
> #ifdef CONFIG_PM_SLEEP
> static void fw_name_devm_release(struct device *dev, void *res)
> {
> @@ -1120,6 +1294,22 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device,
> return 0;
> }
>
> +#ifdef CONFIG_FIRMWARE_SIG
> +static void fw_check_sig_ok(const struct firmware *fw, const char *name)
> +{
> + struct firmware_buf *buf = fw->priv;
> +
> + if (!buf->sig_ok)
> + pr_notice_once("%s: firmware verification failed: signature "
> + "and/or required key missing\n", name);
> +}
> +#else
> +static void fw_check_sig_ok(const struct firmware *fw, const char *name)
> +{
> + return;
> +}
> +#endif
> +
> /* called from request_firmware() and request_firmware_work_func() */
> static int
> _request_firmware(const struct firmware **firmware_p, const char *name,
> @@ -1177,6 +1367,15 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
> usermodehelper_read_unlock();
>
> out:
> + if (ret >= 0) {
> + ret = firmware_sig_check(fw, name);
> + if (ret)
> + goto out_bad_sig;
> + fw_check_sig_ok(fw, name);
> + }
> +
> + out_bad_sig:
> +
> if (ret < 0) {
> release_firmware(fw);
> fw = NULL;
> --
> 2.3.2.209.gd67f9d5.dirty
>
Thanks,
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists