[RFC v2 0/6] firmware: add PKCS#7 firmware signature support

[RFC v2 0/6] firmware: add PKCS#7 firmware signature support

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Upon review of the v1 series David Howells requested I re-work
the code on top of his PKCS#7 branch [1] which moves module signing
to support and use PKCS#7. This v2 series is based on top of that
branch. Other than David's own changes this series depends on some
other changes Rusty has taken in already but not yet visible on
linux-next:

kernel/params: constify struct kernel_param_ops uses
kernel/module.c: use generic module param operaters for sig_enforce
kernel/params.c: generalize bool_enable_only
moduleparam.h: add module_param_config_*() helpers
kernel/workqueue.c: remove ifdefs over wq_power_efficient
kernel/workqueue.c: use module_param_config_on_off() for power_efficient
kernel/module.c: avoid ifdefs for sig_enforce declaration
kernel/params.c: export param_ops_bool_enable_only

This series also depends on the fixes recently posted for the firmware_class
driver:

firmware: fix __getname() missing failure check
firmware: check for file truncation on direct firmware loading
firmware: fix possible use after free on name on asynchronous request
firmware: use const for remaining firmware names

Since that is quite a bit of delta, if folks want to help test / review
this on a tree, you can use this tree with the fw-signing-v2-20150513
branch:

https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux.git/

This v2 modifies the way we do firmware signature checking from the v1 RFCs by
using separate detached files for the signatures of the firmware files, as
suggested by David. If you have foo.bin, you'll need foo.bin.pkcs7 present
when firmware signature checks are enabled.

Since it is based on David's modsign-pkcs7 branch and since these
are not yet upstream I think its best if David took what he can as part of his
series and squashed commits when and where possible. For instance the
sign-file.c changes can likely be squashed. Patch 2, which generalizes module
signing as system data signing is based on David's own series, in my previous
series I addressed code in place upstream which lacked PKCS#7 support, I'll
leave it up to David to decide if / when to merge this into his series but
I think that if this is reasonable it can go in prior to addition of
PKCS#7 support, in which case my v1 patch can be used. Otherwise this v2
will need to be used. Also, patch 2 in this v2 series changes the config
SYSDATA_SIG to def_bool n as noted by Paul Bolle.

Patch 3 simply has the commit log massaged to account for the discussions
on the recursive issue it tries to fix. Based on discussions the recursive
issue reported by the code is real [2], however since FW_LOADER is an EXPERT
option it is reasonable to make this change anyway. A change for now seems
reasonable.

Lastly, we'll need to export data_verify_pkcs7() as we are dealing with
a split between that data and signature. The last patch adds firmware
signature support. I like to think it works as expected, with a simple
requirement to have you sign-file -s the binaries. As with module signing
there are 3 modes of operation with firmware signature support:

  a) signature disabled
  b) signature enabled - but permissive - we'll let things slide if not signed
  c) signature enforced - firmware not signed will not be allowed

In the permissive case though the only difference from the module
signing code / logic is that we *do not* taint the kernel. The reason
for this is technical. First of all add_taint_module() is not exported,
are we OK to export it? If not what users do we wish to grant access to
it? If we wish to provide a mechanism by which we can have a say in that
the old module namespace work by Andi Kleen comes to mind as useful for
that [3]. If we're OK with wide access to add_taint_kernel() as an exported
symbol there are yet other technical questions to address, add_taint_kernel()
requires a module passed and only *one* of the 3 request_firmware*() APIs peg
the correct module caller of the API onto firmware_class, the others use the
firmware_class struct module.

We have a few options then:

  1) Extend the firmware_class driver API to require the correct module to
     always be set. Doing this IMHO is worthy, but its not just worthy for
     firmware signing tainting, it may come in handy in the future. There's
     an issue with extending the firmware_class APIs though which make me
     nervous about just doing this change even if we wanted this alone for
     firmware signature checks -- the firmware_class APIs keep being extended
     requiring collateral evolutions which IMHO can be avoided if we sat and
     thought about a decent API which can be grown for our requirements.

  2) Use add_taint_module() with the firmware_class module for the two
     synchrounous APIs, while using the right module for the async call.

  3) Use the firmware_class module for all 3 API calls when using
     add_taint_module()

  4) Skip tainting the kernel for unsigned firmware files

The approach I've decided to take here is a combination of 4) and 1). I've
decided that extending the firmware_class API even more is not a good idea
anymore. Part of this is because I have some other uses cases for
firmware_class's use and having the right module is not the only thing I'd
want to extend firmware_class with support for. I address a spring cleaning
of the firmware_class API in my next series, while also enabling passing
the right module always, and enabling use of tainting. I think its best to
leave the old APIs as-is then, and only if folks really require a shift to
taint they can then consider the new APIs. If we *really* want to taint the
kernel even for the old APIs I recommend just dealing with using the
firmware_class (THIS_MODULE) for now -- I don't think making the taint
specific to a module for firmware is worth the collateral evolutions required
using and extending the old APIs.

The new extensible firmware_class APIs go in my next series, that enables
tainting the specific caller module, among other things, as you'll see.

[0] https://lkml.org/lkml/2015/5/5/1345
[1] http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
[2] https://lkml.org/lkml/2015/5/5/1353
[3] https://backports.wiki.kernel.org/index.php/Documentation/backports/hacking/todo#Module_namespaces

Luis R. Rodriguez (6):
  firmware: generalize reading file contents as a helper
  kernel: generalize module signing as system data signing
  crypto: qat - address recursive dependency when fw signing is enabled
  scripts/sign-file.c: add support to only create signature file
  kernel/sysdata_signing: export data_verify_pkcs7()
  firmware: add firmware signature checking support

 Documentation/firmware_class/signing.txt           |  88 ++++++++
 drivers/base/Kconfig                               |  18 ++
 drivers/base/firmware_class.c                      | 237 ++++++++++++++++++++-
 drivers/crypto/qat/Kconfig                         |   2 +-
 .../module-internal.h => include/linux/sysdata.h   |   6 +-
 init/Kconfig                                       |  24 ++-
 kernel/Makefile                                    |   2 +-
 kernel/module.c                                    |   4 +-
 kernel/{module_signing.c => sysdata_signing.c}     |  60 +++---
 kernel/system_keyring.c                            |   2 +-
 scripts/sign-file.c                                |  17 +-
 11 files changed, 399 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/firmware_class/signing.txt
 rename kernel/module-internal.h => include/linux/sysdata.h (64%)
 rename kernel/{module_signing.c => sysdata_signing.c} (63%)

-- 
2.3.2.209.gd67f9d5.dirty

[RFC v2 1/6] firmware: generalize reading file contents as a helper

[Luis R. Rodriguez]

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>
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

[RFC v2 2/6] kernel: generalize module signing as system data signing

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This generalizes the module signing code as helpers, we do
this as we'll later re-use this same code for firmware and
other system data signing.

Acked-by: Rusty Russell <rusty@rustcorp.com.au> (module parts)
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>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 init/Kconfig                                     | 24 ++++++----
 kernel/Makefile                                  |  2 +-
 kernel/module.c                                  |  4 +-
 kernel/{module-internal.h => sysdata-internal.h} |  4 +-
 kernel/{module_signing.c => sysdata_signing.c}   | 58 ++++++++++++------------
 kernel/system_keyring.c                          |  2 +-
 6 files changed, 49 insertions(+), 45 deletions(-)
 rename kernel/{module-internal.h => sysdata-internal.h} (79%)
 rename kernel/{module_signing.c => sysdata_signing.c} (64%)

diff --git a/init/Kconfig b/init/Kconfig
index fb98cba..a75c587 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1793,6 +1793,19 @@ config BASE_SMALL
 	default 0 if BASE_FULL
 	default 1 if !BASE_FULL
 
+config SYSDATA_SIG
+	def_bool n
+	select SYSTEM_TRUSTED_KEYRING
+	select KEYS
+	select CRYPTO
+	select ASYMMETRIC_KEY_TYPE
+	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select PUBLIC_KEY_ALGO_RSA
+	select ASN1
+	select OID_REGISTRY
+	select X509_CERTIFICATE_PARSER
+	select PKCS7_MESSAGE_PARSER
+
 menuconfig MODULES
 	bool "Enable loadable module support"
 	option modules
@@ -1866,16 +1879,7 @@ config MODULE_SRCVERSION_ALL
 config MODULE_SIG
 	bool "Module signature verification"
 	depends on MODULES
-	select SYSTEM_TRUSTED_KEYRING
-	select KEYS
-	select CRYPTO
-	select ASYMMETRIC_KEY_TYPE
-	select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-	select PUBLIC_KEY_ALGO_RSA
-	select ASN1
-	select OID_REGISTRY
-	select X509_CERTIFICATE_PARSER
-	select PKCS7_MESSAGE_PARSER
+	select SYSDATA_SIG
 	help
 	  Check modules for valid signatures upon load: the signature
 	  is simply appended to the module. For more information see
diff --git a/kernel/Makefile b/kernel/Makefile
index 60c302c..ed6a32b 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -47,7 +47,7 @@ endif
 obj-$(CONFIG_UID16) += uid16.o
 obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o
 obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o
+obj-$(CONFIG_SYSDATA_SIG) += sysdata_signing.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_KEXEC) += kexec.o
diff --git a/kernel/module.c b/kernel/module.c
index 9e51b37..6a3f629 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -60,7 +60,7 @@
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
 #include <uapi/linux/module.h>
-#include "module-internal.h"
+#include "sysdata-internal.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
@@ -2404,7 +2404,7 @@ static int module_sig_check(struct load_info *info)
 	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
 		/* We truncate the module to discard the signature */
 		info->len -= markerlen;
-		err = mod_verify_sig(mod, &info->len);
+		err = sysdata_verify_sig(mod, &info->len);
 	}
 
 	if (!err) {
diff --git a/kernel/module-internal.h b/kernel/sysdata-internal.h
similarity index 79%
rename from kernel/module-internal.h
rename to kernel/sysdata-internal.h
index 915e123..0aa573e 100644
--- a/kernel/module-internal.h
+++ b/kernel/sysdata-internal.h
@@ -1,4 +1,4 @@
-/* Module internals
+/* System Data internals
  *
  * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@redhat.com)
@@ -9,4 +9,4 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
-extern int mod_verify_sig(const void *mod, unsigned long *_modlen);
+extern int sysdata_verify_sig(const void *data, unsigned long *_len);
diff --git a/kernel/module_signing.c b/kernel/sysdata_signing.c
similarity index 64%
rename from kernel/module_signing.c
rename to kernel/sysdata_signing.c
index 8eb20cc..adc44d4 100644
--- a/kernel/module_signing.c
+++ b/kernel/sysdata_signing.c
@@ -1,4 +1,4 @@
-/* Module signature checker
+/* System Data signature checker
  *
  * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@redhat.com)
@@ -14,10 +14,10 @@
 #include <keys/system_keyring.h>
 #include <crypto/public_key.h>
 #include <crypto/pkcs7.h>
-#include "module-internal.h"
+#include "sysdata-internal.h"
 
 /*
- * Module signature information block.
+ * System Data signature information block.
  *
  * The constituents of the signature section are, in order:
  *
@@ -26,7 +26,7 @@
  *	- Signature data
  *	- Information block
  */
-struct module_signature {
+struct sysdata_signature {
 	u8	algo;		/* Public-key crypto algorithm [0] */
 	u8	hash;		/* Digest algorithm [0] */
 	u8	id_type;	/* Key identifier type [PKEY_ID_PKCS7] */
@@ -37,10 +37,10 @@ struct module_signature {
 };
 
 /*
- * Verify a PKCS#7-based signature on a module.
+ * Verify a PKCS#7-based signature on system data.
  */
-static int mod_verify_pkcs7(const void *mod, unsigned long modlen,
-			    const void *raw_pkcs7, size_t pkcs7_len)
+static int data_verify_pkcs7(const void *data, unsigned long len,
+			     const void *raw_pkcs7, size_t pkcs7_len)
 {
 	struct pkcs7_message *pkcs7;
 	bool trusted;
@@ -51,7 +51,7 @@ static int mod_verify_pkcs7(const void *mod, unsigned long modlen,
 		return PTR_ERR(pkcs7);
 
 	/* The data should be detached - so we need to supply it. */
-	if (pkcs7_supply_detached_data(pkcs7, mod, modlen) < 0) {
+	if (pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
 		pr_err("PKCS#7 signature with non-detached data\n");
 		ret = -EBADMSG;
 		goto error;
@@ -77,42 +77,42 @@ error:
 }
 
 /*
- * Verify the signature on a module.
+ * Verify the signature on system data.
  */
-int mod_verify_sig(const void *mod, unsigned long *_modlen)
+int sysdata_verify_sig(const void *data, unsigned long *_len)
 {
-	struct module_signature ms;
-	size_t modlen = *_modlen, sig_len;
+	struct sysdata_signature ds;
+	size_t len = *_len, sig_len;
 
-	pr_devel("==>%s(,%zu)\n", __func__, modlen);
+	pr_devel("==>%s(,%zu)\n", __func__, len);
 
-	if (modlen <= sizeof(ms))
+	if (len <= sizeof(ds))
 		return -EBADMSG;
 
-	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
-	modlen -= sizeof(ms);
+	memcpy(&ds, data + (len - sizeof(ds)), sizeof(ds));
+	len -= sizeof(ds);
 
-	sig_len = be32_to_cpu(ms.sig_len);
-	if (sig_len >= modlen)
+	sig_len = be32_to_cpu(ds.sig_len);
+	if (sig_len >= len)
 		return -EBADMSG;
-	modlen -= sig_len;
-	*_modlen = modlen;
+	len -= sig_len;
+	*_len = len;
 
-	if (ms.id_type != PKEY_ID_PKCS7) {
+	if (ds.id_type != PKEY_ID_PKCS7) {
 		pr_err("Module is not signed with expected PKCS#7 message\n");
 		return -ENOPKG;
 	}
 
-	if (ms.algo != 0 ||
-	    ms.hash != 0 ||
-	    ms.signer_len != 0 ||
-	    ms.key_id_len != 0 ||
-	    ms.__pad[0] != 0 ||
-	    ms.__pad[1] != 0 ||
-	    ms.__pad[2] != 0) {
+	if (ds.algo != 0 ||
+	    ds.hash != 0 ||
+	    ds.signer_len != 0 ||
+	    ds.key_id_len != 0 ||
+	    ds.__pad[0] != 0 ||
+	    ds.__pad[1] != 0 ||
+	    ds.__pad[2] != 0) {
 		pr_err("PKCS#7 signature info has unexpected non-zero params\n");
 		return -EBADMSG;
 	}
 
-	return mod_verify_pkcs7(mod, modlen, mod + modlen, sig_len);
+	return data_verify_pkcs7(data, len, data + len, sig_len);
 }
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 875f64e..1eb0c86 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -16,7 +16,7 @@
 #include <linux/err.h>
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
-#include "module-internal.h"
+#include "sysdata-internal.h"
 
 struct key *system_trusted_keyring;
 EXPORT_SYMBOL_GPL(system_trusted_keyring);
-- 
2.3.2.209.gd67f9d5.dirty

[RFC v2 3/6] crypto: qat - address recursive dependency when fw signing is enabled

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

We're going to add firmware module signing support, but when we do
this we end up with the following recursive dependency. Fix this by
just depending on FW_LOADER, which is typically always enabled
anyway. We do this as a compromise for now as FW_LOADER is only
selectable when EXPERT is enabled and the recursive dependency issue
below is actually real and valid, its proper fix however requires
a bit of work.

This technically means this change is likely welcomed for other
parts of the kernel but this is really only needed for now here.

mcgrof@ergon ~/linux-next (git::master)$ make allnoconfig
scripts/kconfig/conf  --allnoconfig Kconfig
crypto/Kconfig:15:error: recursive dependency detected!
crypto/Kconfig:15:      symbol CRYPTO is selected by SYSDATA_SIG
init/Kconfig:1880:      symbol SYSDATA_SIG is selected by FIRMWARE_SIG
drivers/base/Kconfig:88:        symbol FIRMWARE_SIG depends on FW_LOADER
drivers/base/Kconfig:80:        symbol FW_LOADER is selected by CRYPTO_DEV_QAT
drivers/crypto/qat/Kconfig:1:   symbol CRYPTO_DEV_QAT is selected by CRYPTO_DEV_QAT_DH895xCC
drivers/crypto/qat/Kconfig:13:  symbol CRYPTO_DEV_QAT_DH895xCC depends on CRYPTO

Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Kyle McMartin <kyle@kernel.org>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Tadeusz Struk <tadeusz.struk@intel.com>
Cc: John Griffin <john.griffin@intel.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/crypto/qat/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/Kconfig b/drivers/crypto/qat/Kconfig
index 49bede2..9bea338 100644
--- a/drivers/crypto/qat/Kconfig
+++ b/drivers/crypto/qat/Kconfig
@@ -8,7 +8,7 @@ config CRYPTO_DEV_QAT
 	select CRYPTO_SHA1
 	select CRYPTO_SHA256
 	select CRYPTO_SHA512
-	select FW_LOADER
+	depends on FW_LOADER
 
 config CRYPTO_DEV_QAT_DH895xCC
 	tristate "Support for Intel(R) DH895xCC"
-- 
2.3.2.209.gd67f9d5.dirty

[RFC v2 4/6] scripts/sign-file.c: add support to only create signature file

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

You can create a file.pkcs7 file with the -d option right now
but that still modifies the file. If all you want is the signature
file you can use -s now, that will leave the file passed as-is.

This will be useful when firmware signature support is added
upstream as firmware will be left intact, and we'll only require
the signature file. The descriptor is implicit by file extension
and the file's own size.

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 scripts/sign-file.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 5b8a6dd..b468f73 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -41,7 +41,7 @@ static __attribute__((noreturn))
 void format(void)
 {
 	fprintf(stderr,
-		"Usage: scripts/sign-file [-dp] <hash algo> <key> <x509> <module> [<dest>]\n");
+		"Usage: scripts/sign-file [-dps] <hash algo> <key> <x509> <module> [<dest>]\n");
 	exit(2);
 }
 
@@ -86,22 +86,24 @@ int main(int argc, char **argv)
 	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;
 	const EVP_MD *digest_algo;
 	EVP_PKEY *private_key;
 	PKCS7 *pkcs7;
 	X509 *x509;
-	BIO *b, *bd, *bm;
+	BIO *b, *bd=NULL, *bm;
 	int opt, n;
 
 	ERR_load_crypto_strings();
 	ERR_clear_error();
 
 	do {
-		opt = getopt(argc, argv, "dp");
+		opt = getopt(argc, argv, "dps");
 		switch (opt) {
 		case 'p': save_pkcs7 = true; break;
+		case 's': sign_only = true; save_pkcs7 = true; break;
 		case -1: break;
 		default: format();
 		}
@@ -148,8 +150,10 @@ int main(int argc, char **argv)
 	/* Open the destination file now so that we can shovel the module data
 	 * across as we read it.
 	 */
-	bd = BIO_new_file(dest_name, "wb");
-	ERR(!bd, "%s", dest_name);
+	if (!sign_only) {
+		bd = BIO_new_file(dest_name, "wb");
+		ERR(!bd, "%s", dest_name);
+	}
 
 	/* Digest the module data. */
 	OpenSSL_add_all_digests();
@@ -180,6 +184,9 @@ int main(int argc, char **argv)
 		BIO_free(b);
 	}
 
+	if (sign_only)
+		return 0;
+
 	/* Append the marker and the PKCS#7 message to the destination file */
 	ERR(BIO_reset(bm) < 0, "%s", module_name);
 	while ((n = BIO_read(bm, buf, sizeof(buf))),
-- 
2.3.2.209.gd67f9d5.dirty

[RFC v2 5/6] kernel/sysdata_signing: export data_verify_pkcs7()

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This will be used for firmware signature check support.

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>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 kernel/sysdata-internal.h | 2 ++
 kernel/sysdata_signing.c  | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/sysdata-internal.h b/kernel/sysdata-internal.h
index 0aa573e..52a5c42 100644
--- a/kernel/sysdata-internal.h
+++ b/kernel/sysdata-internal.h
@@ -10,3 +10,5 @@
  */
 
 extern int sysdata_verify_sig(const void *data, unsigned long *_len);
+int data_verify_pkcs7(const void *data, unsigned long len,
+		      const void *raw_pkcs7, size_t pkcs7_len);
diff --git a/kernel/sysdata_signing.c b/kernel/sysdata_signing.c
index adc44d4..6759c54 100644
--- a/kernel/sysdata_signing.c
+++ b/kernel/sysdata_signing.c
@@ -39,8 +39,8 @@ struct sysdata_signature {
 /*
  * Verify a PKCS#7-based signature on system data.
  */
-static int data_verify_pkcs7(const void *data, unsigned long len,
-			     const void *raw_pkcs7, size_t pkcs7_len)
+int data_verify_pkcs7(const void *data, unsigned long len,
+		      const void *raw_pkcs7, size_t pkcs7_len)
 {
 	struct pkcs7_message *pkcs7;
 	bool trusted;
@@ -75,6 +75,7 @@ error:
 	pr_devel("<==%s() = %d\n", __func__, ret);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(data_verify_pkcs7);
 
 /*
  * Verify the signature on system data.
-- 
2.3.2.209.gd67f9d5.dirty

[RFC v2 6/6] firmware: add firmware signature checking support

[Luis R. Rodriguez]

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
mechanisms on userspace data needed.

As with module signing, we do a very simple search for a
particular string appended to the firmware. 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, the kernel will be tainted
if a firmware is loaded that is unsigned or has a signature
for which we don't have the key.

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>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 Documentation/firmware_class/signing.txt           |  88 +++++++++
 drivers/base/Kconfig                               |  18 ++
 drivers/base/firmware_class.c                      | 214 ++++++++++++++++++++-
 .../sysdata-internal.h => include/linux/sysdata.h  |   0
 kernel/module.c                                    |   2 +-
 kernel/sysdata_signing.c                           |   3 +-
 kernel/system_keyring.c                            |   2 +-
 7 files changed, 317 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/firmware_class/signing.txt
 rename kernel/sysdata-internal.h => include/linux/sysdata.h (100%)

diff --git a/Documentation/firmware_class/signing.txt b/Documentation/firmware_class/signing.txt
new file mode 100644
index 0000000..6e1ce3c
--- /dev/null
+++ b/Documentation/firmware_class/signing.txt
@@ -0,0 +1,88 @@
+			================================
+			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.pkcs7. When firmware signature
+checking is enabled (FIRMWARE_SIG) when one of the above APIs is used
+against foo.bin, the file foo.bin.pkcs7 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.pkcs7
+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 unsiged 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", under the same menu.
+
+==================
+USING SIGNING KEYS
+==================
+
+For details on the types of keys used, allowed, and how to generate them
+refer to Documentation/module-signing.txt. We end up with two keys:
+
+	signing_key.priv
+	signing_key.x509
+
+======================
+SIGNING FIRMWARE FILES
+======================
+
+To sign a firmware, use the scripts/sign-file tool available in
+the Linux kernel source tree.  The script requires 4 arguments:
+
+	1.  The hash algorithm (e.g., sha256)
+	2.  The private key filename
+	3.  The public key filename
+	4.  The firmware file to be signed
+
+We want to sign the firmware file as a new detached file, for that be
+sure to use the -s flag. The following is an example of how to sign a
+kernel firmware file:
+
+	scripts/sign-file -s sha512 kernel-signkey.priv \
+		kernel-signkey.x509 /lib/firmware/foo.bin
+
+That should have created for you a /lib/firmware/foo.bin.pkcs7 file.
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 98504ec..a1a6db1 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.pkcs7 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..f1d0a41 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 <linux/sysdata.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 = ".pkcs7";
+	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,43 @@ 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 = data_verify_pkcs7(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 +1295,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 +1368,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;
diff --git a/kernel/sysdata-internal.h b/include/linux/sysdata.h
similarity index 100%
rename from kernel/sysdata-internal.h
rename to include/linux/sysdata.h
diff --git a/kernel/module.c b/kernel/module.c
index 6a3f629..0a97256 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -59,8 +59,8 @@
 #include <linux/jump_label.h>
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
+#include <linux/sysdata.h>
 #include <uapi/linux/module.h>
-#include "sysdata-internal.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
diff --git a/kernel/sysdata_signing.c b/kernel/sysdata_signing.c
index 6759c54..32d5f92 100644
--- a/kernel/sysdata_signing.c
+++ b/kernel/sysdata_signing.c
@@ -11,10 +11,10 @@
 
 #include <linux/kernel.h>
 #include <linux/err.h>
+#include <linux/sysdata.h>
 #include <keys/system_keyring.h>
 #include <crypto/public_key.h>
 #include <crypto/pkcs7.h>
-#include "sysdata-internal.h"
 
 /*
  * System Data signature information block.
@@ -117,3 +117,4 @@ int sysdata_verify_sig(const void *data, unsigned long *_len)
 
 	return data_verify_pkcs7(data, len, data + len, sig_len);
 }
+EXPORT_SYMBOL_GPL(sysdata_verify_sig);
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index 1eb0c86..a0b8653 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -14,9 +14,9 @@
 #include <linux/sched.h>
 #include <linux/cred.h>
 #include <linux/err.h>
+#include <linux/sysdata.h>
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
-#include "sysdata-internal.h"
 
 struct key *system_trusted_keyring;
 EXPORT_SYMBOL_GPL(system_trusted_keyring);
-- 
2.3.2.209.gd67f9d5.dirty

Re: [RFC v2 6/6] firmware: add firmware signature checking support

[Luis R. Rodriguez]

On Wed, May 13, 2015 at 11:23:56AM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> As with module signing, we do a very simple search for a
> particular string appended to the firmware. 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, the kernel will be tainted
> if a firmware is loaded that is unsigned or has a signature
> for which we don't have the key.

Sorry this commit log is obviously still from the v1, the cover
letter addresses the changes best...

 Luis

Re: [RFC v2 6/6] firmware: add firmware signature checking support

[Julian Calaby]

Hi Luis,

On Thu, May 14, 2015 at 4:23 AM, 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
> mechanisms on userspace data needed.
>
> As with module signing, we do a very simple search for a
> particular string appended to the firmware. 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, the kernel will be tainted
> if a firmware is loaded that is unsigned or has a signature
> for which we don't have the key.
>
> 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>
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  Documentation/firmware_class/signing.txt           |  88 +++++++++
>  drivers/base/Kconfig                               |  18 ++
>  drivers/base/firmware_class.c                      | 214 ++++++++++++++++++++-
>  .../sysdata-internal.h => include/linux/sysdata.h  |   0
>  kernel/module.c                                    |   2 +-
>  kernel/sysdata_signing.c                           |   3 +-
>  kernel/system_keyring.c                            |   2 +-
>  7 files changed, 317 insertions(+), 10 deletions(-)
>  create mode 100644 Documentation/firmware_class/signing.txt
>  rename kernel/sysdata-internal.h => include/linux/sysdata.h (100%)
>
> diff --git a/Documentation/firmware_class/signing.txt b/Documentation/firmware_class/signing.txt
> new file mode 100644
> index 0000000..6e1ce3c
> --- /dev/null
> +++ b/Documentation/firmware_class/signing.txt
> @@ -0,0 +1,88 @@
> +                       ================================
> +                       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.pkcs7. When firmware signature
> +checking is enabled (FIRMWARE_SIG) when one of the above APIs is used
> +against foo.bin, the file foo.bin.pkcs7 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.pkcs7
> +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 unsiged 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", under the same menu.

You reference the relevant Kconfig symbols above, do you want to add
it here too?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [RFC v2 6/6] firmware: add firmware signature checking support

[Luis R. Rodriguez]

On Thu, May 14, 2015 at 10:31:52AM +1000, Julian Calaby wrote:
> On Thu, May 14, 2015 at 4:23 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > +"Require all firmware to be validly signed", under the same menu.
> 
> You reference the relevant Kconfig symbols above, do you want to add
> it here too?

Sure, amended, thanks.

 Luis

Re: [RFC v2 3/6] crypto: qat - address recursive dependency when fw signing is enabled

[Herbert Xu]

On Wed, May 13, 2015 at 11:23:53AM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> We're going to add firmware module signing support, but when we do
> this we end up with the following recursive dependency. Fix this by
> just depending on FW_LOADER, which is typically always enabled
> anyway. We do this as a compromise for now as FW_LOADER is only
> selectable when EXPERT is enabled and the recursive dependency issue
> below is actually real and valid, its proper fix however requires
> a bit of work.
> 
> This technically means this change is likely welcomed for other
> parts of the kernel but this is really only needed for now here.

Nack.  Either fix kbuild or make every user of FW_LOADER use
a dependency.  Singling out a single crypto device makes zero
sense.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC v2 4/6] scripts/sign-file.c: add support to only create signature file

[David Howells]

Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:

> You can create a file.pkcs7 file with the -d option right now
> but that still modifies the file. If all you want is the signature
> file you can use -s now, that will leave the file passed as-is.

I would recommend you use something other than "-s" - that belongs to the code
that is temporarily removed from the old perl script that allowed an external
signature to be passed.

David

Re: [RFC v2 4/6] scripts/sign-file.c: add support to only create signature file

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> 
> > You can create a file.pkcs7 file with the -d option right now
> > but that still modifies the file. If all you want is the signature
> > file you can use -s now, that will leave the file passed as-is.
> 
> I would recommend you use something other than "-s" - that belongs to the code
> that is temporarily removed from the old perl script that allowed an external
> signature to be passed.

Use "-d".  I added the flag but then never made it do anything.

David

Re: [RFC v2 4/6] scripts/sign-file.c: add support to only create signature file

[Luis R. Rodriguez]

On Thu, May 14, 2015 at 7:52 AM, David Howells <dhowells@redhat.com> wrote:
> Use "-d".  I added the flag but then never made it do anything.

Sure, but since this C file is not upstream, how about just squashing
the commit with yours that adds the C file?

 Luis

Re: [RFC v2 4/6] scripts/sign-file.c: add support to only create signature file

[David Howells]

Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:

> > Use "-d".  I added the flag but then never made it do anything.
> 
> Sure, but since this C file is not upstream, how about just squashing
> the commit with yours that adds the C file?

I've got various Tested-by's on that.

Anyway, I'm going to stack the attached patch on my branch if that's okay by
you...

David
---
commit 092c720a627b913918eb22f6700f189b34f70693
Author: Luis R. Rodriguez <mcgrof@suse.com>
Date:   Wed May 13 11:23:54 2015 -0700

    sign-file: Add option to only create signature file
    
    Make the -d option (which currently isn't actually wired to anything) write
    out the PKCS#7 message as per the -p option and then exit without either
    modifying the source or writing out a compound file of the source, signature
    and metadata.
    
    This will be useful when firmware signature support is added
    upstream as firmware will be left intact, and we'll only require
    the signature file. The descriptor is implicit by file extension
    and the file's own size.
    
    Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 5b8a6dda3235..39aaabe89388 100755
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -86,13 +86,14 @@ int main(int argc, char **argv)
 	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;
 	const EVP_MD *digest_algo;
 	EVP_PKEY *private_key;
 	PKCS7 *pkcs7;
 	X509 *x509;
-	BIO *b, *bd, *bm;
+	BIO *b, *bd = NULL, *bm;
 	int opt, n;
 
 	ERR_load_crypto_strings();
@@ -102,6 +103,7 @@ int main(int argc, char **argv)
 		opt = getopt(argc, argv, "dp");
 		switch (opt) {
 		case 'p': save_pkcs7 = true; break;
+		case 'd': sign_only = true; save_pkcs7 = true; break;
 		case -1: break;
 		default: format();
 		}
@@ -148,8 +150,10 @@ int main(int argc, char **argv)
 	/* Open the destination file now so that we can shovel the module data
 	 * across as we read it.
 	 */
-	bd = BIO_new_file(dest_name, "wb");
-	ERR(!bd, "%s", dest_name);
+	if (!sign_only) {
+		bd = BIO_new_file(dest_name, "wb");
+		ERR(!bd, "%s", dest_name);
+	}
 
 	/* Digest the module data. */
 	OpenSSL_add_all_digests();
@@ -180,6 +184,9 @@ int main(int argc, char **argv)
 		BIO_free(b);
 	}
 
+	if (sign_only)
+		return 0;
+
 	/* Append the marker and the PKCS#7 message to the destination file */
 	ERR(BIO_reset(bm) < 0, "%s", module_name);
 	while ((n = BIO_read(bm, buf, sizeof(buf))),

Re: [RFC v2 4/6] scripts/sign-file.c: add support to only create signature file

[Luis R. Rodriguez]

On Thu, May 14, 2015 at 8:02 AM, David Howells <dhowells@redhat.com> wrote:
> I've got various Tested-by's on that.

Didn't know that would stop people from asking us from squashing them
but if so I'll try to use that some other time :)

> Anyway, I'm going to stack the attached patch on my branch if that's okay by
> you...

Sure thing. Let me know what you think about patch 2 and 5 as well, as
they might share similar fate.

  Luis

Re: [RFC v2 3/6] crypto: qat - address recursive dependency when fw signing is enabled

[Luis R. Rodriguez]

On Thu, May 14, 2015 at 11:04:15AM +0800, Herbert Xu wrote:
> On Wed, May 13, 2015 at 11:23:53AM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > We're going to add firmware module signing support, but when we do
> > this we end up with the following recursive dependency. Fix this by
> > just depending on FW_LOADER, which is typically always enabled
> > anyway. We do this as a compromise for now as FW_LOADER is only
> > selectable when EXPERT is enabled and the recursive dependency issue
> > below is actually real and valid, its proper fix however requires
> > a bit of work.
> > 
> > This technically means this change is likely welcomed for other
> > parts of the kernel but this is really only needed for now here.
> 
> Nack.  Either fix kbuild or make every user of FW_LOADER use
> a dependency.  Singling out a single crypto device makes zero
> sense.

I think that's fair... Paul, did you get to look into the
recursive issue some more by any chance? Any preference for
what path to take here?

  Luis