* [RFC v3 1/2] firmware: generalize reading file contents as a helper
2015-05-19 0:45 [RFC v3 0/2] firmware: add PKCS#7 firmware signature support Luis R. Rodriguez
@ 2015-05-19 0:45 ` Luis R. Rodriguez
2015-05-19 0:45 ` [RFC v3 2/2] firmware: add firmware signature checking support Luis R. Rodriguez
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2015-05-19 0:45 UTC (permalink / raw)
To: ming.lei
Cc: rusty, torvalds, dhowells, seth.forshee, linux-kernel, pebolle,
linux-wireless, gregkh, jlee, tiwai, casey, keescook, mjg59,
akpm, Luis R. Rodriguez, Kyle McMartin, David Woodhouse
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We'll want to reuse this same code later in order to
read two separate types of file contents. Although we
can simplify fw_read_file_contents() to do a direct
return we leave a bit of boilerplate code to make the
next changes easier to review. In this case we'll
later extend the firmware specific read to also go
and fetch the signature file when required.
This commit introduces no functional changes.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Kyle McMartin <kyle@kernel.org>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/base/firmware_class.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8c3aa3c..134dd77 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -291,7 +291,8 @@ static const char * const fw_path[] = {
module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
-static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
+static int __read_file_contents(struct file *file,
+ void **dest_buf, size_t *dest_size)
{
int size;
char *buf;
@@ -314,14 +315,30 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
rc = security_kernel_fw_from_file(file, buf, size);
if (rc)
goto fail;
- fw_buf->data = buf;
- fw_buf->size = size;
+
+ *dest_buf = buf;
+ *dest_size = size;
+
return 0;
fail:
vfree(buf);
return rc;
}
+static int fw_read_file_contents(struct file *file,
+ struct firmware_buf *fw_buf)
+{
+ int rc;
+
+ rc = __read_file_contents(file,
+ &fw_buf->data,
+ &fw_buf->size);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+
static int fw_get_filesystem_firmware(struct device *device,
struct firmware_buf *buf)
{
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC v3 2/2] firmware: add firmware signature checking support
2015-05-19 0:45 [RFC v3 0/2] firmware: add PKCS#7 firmware signature support Luis R. Rodriguez
2015-05-19 0:45 ` [RFC v3 1/2] firmware: generalize reading file contents as a helper Luis R. Rodriguez
@ 2015-05-19 0:45 ` Luis R. Rodriguez
2015-06-08 19:56 ` Kees Cook
2015-05-19 9:33 ` David Howells
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Luis R. Rodriguez @ 2015-05-19 0:45 UTC (permalink / raw)
To: ming.lei
Cc: rusty, torvalds, dhowells, seth.forshee, linux-kernel, pebolle,
linux-wireless, gregkh, jlee, tiwai, casey, keescook, mjg59,
akpm, Luis R. Rodriguez, Kyle McMartin, David Woodhouse
From: "Luis R. Rodriguez" <mcgrof@suse.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@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Kyle McMartin <kyle@kernel.org>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.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));
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-05-19 0:45 ` [RFC v3 2/2] firmware: add firmware signature checking support Luis R. Rodriguez
@ 2015-06-08 19:56 ` Kees Cook
2015-07-14 19:20 ` Luis R. Rodriguez
0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2015-06-08 19:56 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Ming Lei, Rusty Russell, Linus Torvalds, David Howells,
seth.forshee, LKML, Paul Bolle, linux-wireless, Greg KH, jlee,
Takashi Iwai, Casey Schaufler, Matthew Garrett, Andrew Morton,
Luis R. Rodriguez, Kyle McMartin, David Woodhouse
On Mon, May 18, 2015 at 5:45 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.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@rustcorp.com.au>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Ming Lei <ming.lei@canonical.com>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Kyle McMartin <kyle@kernel.org>
> Cc: David Woodhouse <David.Woodhouse@intel.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-06-08 19:56 ` Kees Cook
@ 2015-07-14 19:20 ` Luis R. Rodriguez
2015-07-21 23:14 ` Luis R. Rodriguez
0 siblings, 1 reply; 11+ messages in thread
From: Luis R. Rodriguez @ 2015-07-14 19:20 UTC (permalink / raw)
To: Kees Cook, David Howells
Cc: Luis R. Rodriguez, Ming Lei, seth.forshee, Rusty Russell,
Linus Torvalds, David Howells, LKML, Paul Bolle, linux-wireless,
Greg KH, jlee, Takashi Iwai, Casey Schaufler, Matthew Garrett,
Andrew Morton, Kyle McMartin, David Woodhouse
On Mon, Jun 08, 2015 at 12:56:44PM -0700, Kees Cook wrote:
> On Mon, May 18, 2015 at 5:45 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > 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
> > @@ -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?
>
Thanks for the feedback, indeed.
Luis
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-07-14 19:20 ` Luis R. Rodriguez
@ 2015-07-21 23:14 ` Luis R. Rodriguez
0 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2015-07-21 23:14 UTC (permalink / raw)
To: Kees Cook, David Howells
Cc: Luis R. Rodriguez, Ming Lei, Seth Forshee, Rusty Russell,
Linus Torvalds, LKML, Paul Bolle, linux-wireless, Greg KH, jlee,
Takashi Iwai, Casey Schaufler, Matthew Garrett, Andrew Morton,
Kyle McMartin, David Woodhouse
On Tue, Jul 14, 2015 at 12:20 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> > + 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?
>>
>
> Thanks for the feedback, indeed.
Ok I've made this change based on David's tree.
Luis
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-05-19 0:45 [RFC v3 0/2] firmware: add PKCS#7 firmware signature support Luis R. Rodriguez
2015-05-19 0:45 ` [RFC v3 1/2] firmware: generalize reading file contents as a helper Luis R. Rodriguez
2015-05-19 0:45 ` [RFC v3 2/2] firmware: add firmware signature checking support Luis R. Rodriguez
@ 2015-05-19 9:33 ` David Howells
2015-05-19 16:23 ` Luis R. Rodriguez
2015-05-19 10:02 ` David Howells
2015-05-22 15:21 ` [RFC v3 0/2] firmware: add PKCS#7 firmware signature support David Howells
4 siblings, 1 reply; 11+ messages in thread
From: David Howells @ 2015-05-19 9:33 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: dhowells, ming.lei, rusty, torvalds, seth.forshee, linux-kernel,
pebolle, linux-wireless, gregkh, jlee, tiwai, casey, keescook,
mjg59, akpm, Luis R. Rodriguez, Kyle McMartin, David Woodhouse
Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> + -nocerts -md $DIGEST_ALGORITHM -binary > \
> + $(FIRMWARE_BLOB_NAME).p7s
Rather than using '>' it might be worth using '-out' instead.
David
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-05-19 9:33 ` David Howells
@ 2015-05-19 16:23 ` Luis R. Rodriguez
0 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2015-05-19 16:23 UTC (permalink / raw)
To: David Howells
Cc: Luis R. Rodriguez, ming.lei, rusty, torvalds, seth.forshee,
linux-kernel, pebolle, linux-wireless, gregkh, jlee, tiwai,
casey, keescook, mjg59, akpm, Kyle McMartin, David Woodhouse
On Tue, May 19, 2015 at 10:33:45AM +0100, David Howells wrote:
> Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
>
> > + -nocerts -md $DIGEST_ALGORITHM -binary > \
> > + $(FIRMWARE_BLOB_NAME).p7s
>
> Rather than using '>' it might be worth using '-out' instead.
Sure, will amend.
Luis
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-05-19 0:45 [RFC v3 0/2] firmware: add PKCS#7 firmware signature support Luis R. Rodriguez
` (2 preceding siblings ...)
2015-05-19 9:33 ` David Howells
@ 2015-05-19 10:02 ` David Howells
2015-05-19 16:31 ` Luis R. Rodriguez
2015-05-22 15:21 ` [RFC v3 0/2] firmware: add PKCS#7 firmware signature support David Howells
4 siblings, 1 reply; 11+ messages in thread
From: David Howells @ 2015-05-19 10:02 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: dhowells, ming.lei, rusty, torvalds, seth.forshee, linux-kernel,
pebolle, linux-wireless, gregkh, jlee, tiwai, casey, keescook,
mjg59, akpm, Luis R. Rodriguez, Kyle McMartin, David Woodhouse
Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> +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.
I'd rework this paragraph somewhat. How about:
The kernel firmware signing facility enables firmware files on a system to
have associated cryptographic signatures that can be used to validate them.
This uses the same mechanism as is used for module signing.
Firmware signatures are kept in separate files from the actual firmware data
to avoid accidental corruption of the firmware and to avoid licensing issues
from changes.
A firmware signature file consists of a PKCS#7 message containing one or
more cryptographic signatures for the respective firmware file. The
signature file is named for the firmware file to which it corresponds and
must be kept in the same directory. For instance, for the signature file
for a firmware file named foo.bin would be named foo.bin.p7s.
When firmware signature checking is enabled (CONFIG_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 a signature file is found, the kernel's system keyring will be searched
for public keys that can be used to verify the signatures held therein. For
any signature for which a matching key is found, the kernel will attempt to
verify the signature with the key. If verification fails on any signature,
the firmware load will be rejected (with EKEYREJECTED) - even if other
signatures match.
If CONFIG_FIRMWARE_SIG_FORCE is also enabled, then the firmware load will be
rejected (with ENOKEY) if there is no signature file or none of the
signatures match any of the keys in the kernel system keyring. But if at
least one signature matches, then the load will be allowed to proceed.
If CONFIG_FIRMWARE_SIG is not enabled the signature file is ignored
completely.
David
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC v3 2/2] firmware: add firmware signature checking support
2015-05-19 10:02 ` David Howells
@ 2015-05-19 16:31 ` Luis R. Rodriguez
0 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2015-05-19 16:31 UTC (permalink / raw)
To: David Howells
Cc: Luis R. Rodriguez, ming.lei, rusty, torvalds, seth.forshee,
linux-kernel, pebolle, linux-wireless, gregkh, jlee, tiwai,
casey, keescook, mjg59, akpm, Kyle McMartin, David Woodhouse
On Tue, May 19, 2015 at 11:02:44AM +0100, David Howells wrote:
> Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
>
> > +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.
>
> I'd rework this paragraph somewhat. How about:
>
<-- snip -->
Looks sexy, taken word for word, thanks!
Luis
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v3 0/2] firmware: add PKCS#7 firmware signature support
2015-05-19 0:45 [RFC v3 0/2] firmware: add PKCS#7 firmware signature support Luis R. Rodriguez
` (3 preceding siblings ...)
2015-05-19 10:02 ` David Howells
@ 2015-05-22 15:21 ` David Howells
4 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2015-05-22 15:21 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: dhowells, ming.lei, rusty, torvalds, seth.forshee, linux-kernel,
pebolle, linux-wireless, gregkh, jlee, tiwai, casey, keescook,
mjg59, akpm, Luis R. Rodriguez
Assuming we want to tag firmware signatures with the name of the firmware blob
as passed to request_firmware(), the attached patch would provide the PKCS#7
side of the support you need.
David
---
commit ef479eb8e645a95ef172918e4df215ecdc823e42
Author: David Howells <dhowells@redhat.com>
Date: Fri May 22 10:13:37 2015 +0100
PKCS#7: Add an optional authenticated attribute to hold firmware name
Modify the sign-file program to take a "-F <firmware name>" parameter. The
name is a utf8 string that, if given, is inserted in a PKCS#7 authenticated
attribute from where it can be extracted by the kernel. Authenticated
attributes are added to the signature digest.
If the attribute is present, the signature would be assumed to be for
firmware and would not be permitted with module signing or kexec. The name
associated with the attribute would be compared to the name passed to
request_firmware() and the load request would be denied if they didn't
match.
If not present, the signature would be rejected if used for firmware.
One oddity is that the attribute is per-signature, so if a second signature
was added (which PKCS#7 supports), it would have to have the attribute added
separately to that signature also.
Note that the OID I have selected is an unregistered element of the Red Hat
OID space to serve as an example. An OID would need to be allocated for
this purpose.
The kernel then parses this out, saves the string and makes sure the same
string (or lack thereof) is present from all signers. Then when
system_verify_data() is called, it is passed a NULL if the attribute is
expected not to be present and the name from request_firmware() if it is
expected to be present. Verification is rejected if there's a mismatch.
This should probably be combined with tagging the X.509 certificate to
indicate what it can be used for.
Not-yet-signed-off-by: David Howells <dhowells@redhat.com>
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 3bd5a1e4c493..01d19f37437a 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/err.h>
#include <linux/oid_registry.h>
+#include <linux/ctype.h>
#include "public_key.h"
#include "pkcs7_parser.h"
#include "pkcs7-asn1.h"
@@ -29,6 +30,13 @@ struct pkcs7_parse_context {
enum OID last_oid; /* Last OID encountered */
unsigned x509_index;
unsigned sinfo_index;
+ unsigned firmware_name_len;
+ enum {
+ maybe_firmware_sig,
+ is_firmware_sig,
+ isnt_firmware_sig
+ } firmware_sig_state : 8;
+ bool signer_has_firmware_name;
const void *raw_serial;
unsigned raw_serial_size;
unsigned raw_issuer_size;
@@ -73,6 +81,7 @@ void pkcs7_free_message(struct pkcs7_message *pkcs7)
pkcs7->signed_infos = sinfo->next;
pkcs7_free_signed_info(sinfo);
}
+ kfree(pkcs7->firmware_name);
kfree(pkcs7);
}
}
@@ -310,6 +319,8 @@ int pkcs7_sig_note_authenticated_attr(void *context, size_t hdrlen,
const void *value, size_t vlen)
{
struct pkcs7_parse_context *ctx = context;
+ char *p;
+ int i;
pr_devel("AuthAttr: %02x %zu [%*ph]\n", tag, vlen, (unsigned)vlen, value);
@@ -320,6 +331,45 @@ int pkcs7_sig_note_authenticated_attr(void *context, size_t hdrlen,
ctx->sinfo->msgdigest = value;
ctx->sinfo->msgdigest_len = vlen;
return 0;
+
+ case OID_firmwareName:
+ if (tag != ASN1_UTF8STR || vlen == 0)
+ return -EBADMSG;
+
+ /* If there's more than one signature, they must have the same
+ * firmware name.
+ */
+ switch (ctx->firmware_sig_state) {
+ case maybe_firmware_sig:
+ for (i = 0; i < vlen; i++)
+ if (!isprint(((const char *)value)[i]))
+ return -EBADMSG;
+ p = kmalloc(vlen + 1, GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+ memcpy(p, value, vlen);
+ p[vlen] = 0;
+ ctx->msg->firmware_name = p;
+ ctx->firmware_name_len = vlen;
+ ctx->firmware_sig_state = is_firmware_sig;
+ pr_debug("Found firmware name '%s'\n", p);
+ ctx->signer_has_firmware_name = true;
+ return 0;
+
+ case is_firmware_sig:
+ if (ctx->firmware_name_len != vlen ||
+ memcmp(ctx->msg->firmware_name, value, vlen) != 0) {
+ pr_warn("Mismatched firmware names\n");
+ return -EBADMSG;
+ }
+ ctx->signer_has_firmware_name = true;
+ return 0;
+
+ case isnt_firmware_sig:
+ pr_warn("Mismatched presence of firmware name\n");
+ return -EBADMSG;
+ }
+
default:
return 0;
}
@@ -334,12 +384,39 @@ int pkcs7_sig_note_set_of_authattrs(void *context, size_t hdrlen,
{
struct pkcs7_parse_context *ctx = context;
+ /* Make sure we either have all or no firmware names */
+ switch (ctx->firmware_sig_state) {
+ case maybe_firmware_sig:
+ ctx->firmware_sig_state = isnt_firmware_sig;
+ case isnt_firmware_sig:
+ break;
+ case is_firmware_sig:
+ if (!ctx->signer_has_firmware_name) {
+ pr_warn("Mismatched presence of firmware name\n");
+ return -EBADMSG;
+ }
+ ctx->signer_has_firmware_name = false;
+ break;
+ }
+
/* We need to switch the 'CONT 0' to a 'SET OF' when we digest */
ctx->sinfo->authattrs = value - (hdrlen - 1);
ctx->sinfo->authattrs_len = vlen + (hdrlen - 1);
return 0;
}
+/**
+ * pkcs7_get_firmware_name - Get firmware name extension value
+ * @pkcs7: The preparsed PKCS#7 message to access
+ *
+ * Get the value of the firmware name extension if there was one or NULL
+ * otherwise.
+ */
+const char *pkcs7_get_firmware_name(const struct pkcs7_message *pkcs7)
+{
+ return pkcs7->firmware_name;
+}
+
/*
* Note the issuing certificate serial number
*/
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index efc7dc9b8f9c..03a30f8578e2 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -50,6 +50,7 @@ struct pkcs7_message {
struct x509_certificate *certs; /* Certificate list */
struct x509_certificate *crl; /* Revocation list */
struct pkcs7_signed_info *signed_infos;
+ char *firmware_name; /* Firmware name if present */
/* Content Data (or NULL) */
enum OID data_type; /* Type of Data */
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index e235ab4957ee..0999eac6313f 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -22,6 +22,7 @@ extern void pkcs7_free_message(struct pkcs7_message *pkcs7);
extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
const void **_data, size_t *_datalen,
bool want_wrapper);
+extern const char *pkcs7_get_firmware_name(const struct pkcs7_message *pkcs7);
/*
* pkcs7_trust.c
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 9791c907cdb7..30303745f845 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -30,7 +30,8 @@ static inline struct key *get_system_trusted_keyring(void)
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
extern int system_verify_data(const void *data, unsigned long len,
- const void *raw_pkcs7, size_t pkcs7_len);
+ const void *raw_pkcs7, size_t pkcs7_len,
+ const char *firmware_name);
#endif
#endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index c2bbf672b84e..b5e0cc621a8b 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -88,6 +88,10 @@ enum OID {
OID_authorityKeyIdentifier, /* 2.5.29.35 */
OID_extKeyUsage, /* 2.5.29.37 */
+ /* Signing */
+ OID_firmwareName, /* 1.3.6.1.4.1.2312.99.1 */
+ OID_firmwareOnlyKey, /* 1.3.6.1.4.1.2312.99.2 */
+
OID__NR
};
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 70ad463f6df0..9361711897ce 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -72,5 +72,5 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
return -EBADMSG;
}
- return system_verify_data(mod, modlen, mod + modlen, sig_len);
+ return system_verify_data(mod, modlen, mod + modlen, sig_len, NULL);
}
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 95f2dcbc7616..cc2d648d1916 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -113,11 +113,14 @@ late_initcall(load_system_certificate_list);
* @len: Size of @data.
* @raw_pkcs7: The PKCS#7 message that is the signature.
* @pkcs7_len: The size of @raw_pkcs7.
+ * @firmware_name: The required firmware name or NULL.
*/
int system_verify_data(const void *data, unsigned long len,
- const void *raw_pkcs7, size_t pkcs7_len)
+ const void *raw_pkcs7, size_t pkcs7_len,
+ const char *firmware_name)
{
struct pkcs7_message *pkcs7;
+ const char *p7_firmware_name;
bool trusted;
int ret;
@@ -125,6 +128,27 @@ int system_verify_data(const void *data, unsigned long len,
if (IS_ERR(pkcs7))
return PTR_ERR(pkcs7);
+ ret = -EINVAL;
+ p7_firmware_name = pkcs7_get_firmware_name(pkcs7);
+ if (firmware_name) {
+ if (!p7_firmware_name) {
+ pr_err("Expected name '%s' in firmware signature but not present\n",
+ firmware_name);
+ goto error;
+ }
+ if (strcmp(p7_firmware_name, firmware_name) != 0) {
+ pr_err("Expected name '%s' in firmware signature but got '%s'\n",
+ firmware_name, p7_firmware_name);
+ goto error;
+ }
+ } else {
+ if (p7_firmware_name) {
+ pr_err("Unexpected firmware name in signature '%s'\n",
+ p7_firmware_name);
+ goto error;
+ }
+ }
+
/* The data should be detached - so we need to supply it. */
if (pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
pr_err("PKCS#7 signature with non-detached data\n");
diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index ad0aa21bd3ac..c07af47fa57a 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -43,6 +43,8 @@ void format(void)
{
fprintf(stderr,
"Usage: scripts/sign-file [-dp] <hash algo> <key> <x509> <module> [<dest>]\n");
+ fprintf(stderr,
+ " scripts/sign-file [-dp] -F <firmware name> <hash algo> <key> <x509> <firmware> [<dest>]\n");
exit(2);
}
@@ -105,12 +107,14 @@ static int pem_pw_cb(char *buf, int len, int w, void *v)
int main(int argc, char **argv)
{
struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
+ const char *firmware_name = NULL;
char *hash_algo = NULL;
char *private_key_name, *x509_name, *module_name, *dest_name;
bool save_pkcs7 = false, replace_orig;
bool sign_only = false;
unsigned char buf[4096];
unsigned long module_size, pkcs7_size;
+ PKCS7_SIGNER_INFO *signer;
const EVP_MD *digest_algo;
EVP_PKEY *private_key;
PKCS7 *pkcs7;
@@ -125,10 +129,11 @@ int main(int argc, char **argv)
key_pass = getenv("KBUILD_SIGN_PIN");
do {
- opt = getopt(argc, argv, "dp");
+ opt = getopt(argc, argv, "dpF:");
switch (opt) {
case 'p': save_pkcs7 = true; break;
case 'd': sign_only = true; save_pkcs7 = true; break;
+ case 'F': firmware_name = optarg; break;
case -1: break;
default: format();
}
@@ -213,8 +218,28 @@ int main(int argc, char **argv)
PKCS7_NOCERTS | PKCS7_PARTIAL | PKCS7_BINARY | PKCS7_DETACHED | PKCS7_STREAM);
ERR(!pkcs7, "PKCS7_sign");
- ERR(!PKCS7_sign_add_signer(pkcs7, x509, private_key, digest_algo, PKCS7_NOCERTS | PKCS7_BINARY),
- "PKCS7_sign_add_signer");
+ signer = PKCS7_sign_add_signer(pkcs7, x509, private_key, digest_algo, PKCS7_NOCERTS | PKCS7_BINARY);
+ ERR(!signer, "PKCS7_sign_add_signer");
+
+ if (firmware_name) {
+ /* Add an entry into the authenticated attributes to note the
+ * firmware name if this is a firmware signature.
+ */
+ ASN1_UTF8STRING *str;
+ int nid;
+
+ // As an example, use an OID of redhat.99.1 - which is not assigned
+ nid = OBJ_create("1.3.6.1.4.1.2312.99.1", NULL, NULL);
+ ERR(!nid, "OBJ_create");
+
+ str = M_ASN1_UTF8STRING_new();
+ ERR(!str, "M_ASN1_UTF8STRING_new");
+ ERR(!ASN1_STRING_set(str, firmware_name, strlen(firmware_name)),
+ "ASN1_STRING_set");
+ ERR(!PKCS7_add_signed_attribute(signer, nid, V_ASN1_UTF8STRING, str),
+ "PKCS7_add_signed_attribute");
+ }
+
ERR(PKCS7_final(pkcs7, bm, PKCS7_NOCERTS | PKCS7_BINARY) < 0,
"PKCS7_final");
^ permalink raw reply related [flat|nested] 11+ messages in thread