[PATCH 0/4] firmware: signature verification

[PATCH 0/4] firmware: signature verification

[AKASHI Takahiro]

This is my first version of patch series for adding signature
verification to firmware loading.

The original idea and code came from the work by Luis some time ago,
and I migrated it with some improvements to new driver data APIs,
leaving request_firmware() intact.

For details about how it works, please see the commit message of
patch#1 and the document under Documentation/driver-api/firmware.

Please note that patch#3, test script, is still a bit rough-edged,
especially that we have to prepare some data files in advance.
I will try to improve it for better automation.

For you convenience, the patch is available:
https://git.linaro.org/people/takahiro.akashi/linux-aarch64.git
						firmware/signature

AKASHI Takahiro (4):
  firmware: add firmware signing
  scripts: sign-file: add firmware-signing option
  test: firmwware: add signature test to driver_data loader test
  firmware: document signature verification for driver data

 Documentation/driver-api/firmware/driver_data.rst  |   6 +
 .../driver-api/firmware/fallback-mechanisms.rst    |   5 +-
 Documentation/driver-api/firmware/signing.rst      |  81 +++++++
 drivers/base/Kconfig                               |  25 ++
 drivers/base/firmware_class.c                      | 211 +++++++++++++++-
 include/linux/driver_data.h                        |   5 +
 lib/test_driver_data.c                             |  56 ++++-
 scripts/sign-file.c                                |   5 +-
 tools/testing/selftests/firmware/driver_data.sh    | 265 ++++++++++++++++++++-
 9 files changed, 638 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/driver-api/firmware/signing.rst

-- 
2.11.1

[PATCH 1/4] firmware: add firmware signing

[AKASHI Takahiro]

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.

Firmware signing is per-data per device and if this feature is
enabled permissively (by default), both valid and invalid firmware,
which can be unsigned, signed by non-trusted key, or even one
with invalid digest, will be loaded, just leaving a warning message
in the kernel log.

On the othe hand, in enforcing mode, which is enabled by either
a kernel configuration (CONFIG_FIRMWARE_SIG_FORCE) or a module
parameter (fw_sig_enforce), only the verified firmware are
allowed to be loaded.

There is one driver data option, DRIVER_DATA_REQ_NO_SIG_CHECK,
which will skip signature verification check at load time
even in enforcing mode.
This option is solely for non security-sensitive data.
Please also note any firmware loaded with request_firmware()
will not be affected by firmware signing.

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>
[akashi:migrated to driver data APIs]
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/base/Kconfig          |  25 +++++
 drivers/base/firmware_class.c | 211 +++++++++++++++++++++++++++++++++++++++---
 include/linux/driver_data.h   |   5 +
 3 files changed, 229 insertions(+), 12 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 74779ee3d3b1..4c9600437396 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -79,6 +79,31 @@ 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 SYSTEM_DATA_VERIFICATION
+	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 in the same directory.
+
+	  This configuration only affects drivers with driver_data APIs,
+	  disabling DRIVER_DATA_REQ_NO_SIG_CHECK.
+
+	  For more information see Documentation/driver-api/firmware/signing.rst
+
+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 trusted key. Without this, you'll only get a record on kernel
+	  log and yet the firmware will be loaded.
+
+	  This configuration only affects drivers with driver_data APIs,
+	  disabling DRIVER_DATA_REQ_NO_SIG_CHECK.
+
 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 e87e91bcd8f8..590a2a834fec 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -43,6 +43,7 @@
 #include <linux/reboot.h>
 #include <linux/security.h>
 #include <linux/swait.h>
+#include <linux/verification.h>
 
 #include <generated/utsrelease.h>
 
@@ -146,6 +147,9 @@ struct driver_data_params {
  *	o request_firmware_nowait():	__DATA_REQ_FIRMWARE_NOWAIT()
  */
 #define __DATA_REQ_FIRMWARE()						\
+	.req_params = {							\
+		.reqs = DRIVER_DATA_REQ_NO_SIG_CHECK,			\
+	},								\
 	.priv_params = {						\
 		.priv_reqs = DRIVER_DATA_PRIV_REQ_FALLBACK |		\
 			     DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT,	\
@@ -153,7 +157,8 @@ struct driver_data_params {
 
 #define __DATA_REQ_FIRMWARE_DIRECT()					\
 	.req_params = {							\
-		.reqs = DRIVER_DATA_REQ_OPTIONAL,			\
+		.reqs = DRIVER_DATA_REQ_OPTIONAL |			\
+			DRIVER_DATA_REQ_NO_SIG_CHECK,			\
 	},								\
 	.priv_params = {						\
 		.priv_reqs = DRIVER_DATA_PRIV_REQ_FALLBACK |		\
@@ -161,6 +166,9 @@ struct driver_data_params {
 	}
 
 #define __DATA_REQ_FIRMWARE_BUF(buf, size)				\
+	.req_params = {							\
+		.reqs = DRIVER_DATA_REQ_NO_SIG_CHECK,			\
+	},								\
 	.priv_params = {						\
 		.priv_reqs = DRIVER_DATA_PRIV_REQ_FALLBACK |		\
 			     DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT |	\
@@ -177,6 +185,7 @@ struct driver_data_params {
 			.found_cb = NULL,				\
 			.found_ctx = async_ctx,				\
 		},							\
+		.reqs = DRIVER_DATA_REQ_NO_SIG_CHECK,			\
 	},								\
 	.priv_params = {						\
 		.mode = DRIVER_DATA_ASYNC,				\
@@ -204,6 +213,8 @@ struct driver_data_params {
 	(!!((params)->reqs & DRIVER_DATA_REQ_KEEP))
 #define driver_data_param_uses_api(params)	\
 	(!!((params)->reqs & DRIVER_DATA_REQ_USE_API_VERSIONING))
+#define driver_data_param_no_sig_check(params)	\
+	(!!((params)->reqs & DRIVER_DATA_REQ_NO_SIG_CHECK))
 
 #define driver_data_sync_cb(param)   ((params)->cbs.sync.found_cb)
 #define driver_data_sync_ctx(params) ((params)->cbs.sync.found_ctx)
@@ -263,6 +274,11 @@ int driver_data_async_call_api_cb(const struct firmware *driver_data,
 						error);
 }
 
+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
@@ -445,6 +461,11 @@ struct firmware_buf {
 	void *data;
 	size_t size;
 	size_t allocated_size;
+	bool sig_ok;
+#ifdef CONFIG_FIRMWARE_SIG
+	void *sig_data;
+	size_t sig_size;
+#endif
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 	bool is_paged_buf;
 	bool need_uevent;
@@ -611,6 +632,9 @@ static void __fw_free_buf(struct kref *ref)
 #endif
 	if (!buf->allocated_size)
 		vfree(buf->data);
+#ifdef CONFIG_FIRMWARE_SIG
+	vfree(buf->sig_data);
+#endif
 	kfree_const(buf->fw_id);
 	kfree(buf);
 }
@@ -648,13 +672,16 @@ fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf)
 	int i, len;
 	int rc = -ENOENT;
 	char *path;
-	enum kernel_read_file_id id = READING_FIRMWARE;
-	size_t msize = INT_MAX;
+	enum kernel_read_file_id id;
+	loff_t msize;
 
 	/* Already populated data member means we're loading into a buffer */
 	if (buf->data) {
 		id = READING_FIRMWARE_PREALLOC_BUFFER;
 		msize = buf->allocated_size;
+	} else {
+		id = READING_FIRMWARE;
+		msize = LONG_MAX;
 	}
 
 	path = __getname();
@@ -673,7 +700,7 @@ fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf)
 			break;
 		}
 
-		buf->size = 0;
+		/* Load firmware */
 		rc = kernel_read_file_from_path(path, &buf->data, &size, msize,
 						id);
 		if (rc) {
@@ -685,11 +712,38 @@ fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf)
 					 path, rc);
 			continue;
 		}
-		dev_dbg(device, "direct-loading %s\n", buf->fw_id);
 		buf->size = size;
+
+#ifdef CONFIG_FIRMWARE_SIG
+		len = snprintf(path, PATH_MAX, "%s/%s.p7s",
+			       fw_path[i], buf->fw_id);
+		if (len >= PATH_MAX) {
+			rc = -ENAMETOOLONG;
+			break;
+		}
+
+		/* Load signature */
+		rc = kernel_read_file_from_path(path, &buf->sig_data, &size,
+						LONG_MAX, READING_FIRMWARE);
+		if (!rc)
+			buf->sig_size = size;
+		else if (rc == -ENOENT)
+			dev_info(device,
+				"signature of %s doesn't exist\n", buf->fw_id);
+		else
+			dev_warn(device,
+				"loading signature of %s failed (%d)\n",
+				buf->fw_id, rc);
+
+		/* caller should check existence of signature with sig_size */
+		rc = 0;
+#endif /* CONFIG_FIRMWARE_SIG */
+
+		dev_dbg(device, "direct-loading %s\n", buf->fw_id);
 		fw_state_done(&buf->fw_st);
 		break;
 	}
+
 	__putname(path);
 
 	return rc;
@@ -716,9 +770,9 @@ 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=%s\n",
 		 __func__, buf->fw_id, buf, buf->data,
-		 (unsigned int)buf->size);
+		 (unsigned int)buf->size, buf->sig_ok ? "(yes)" : "(no)");
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -822,6 +876,25 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device,
 	return 0;
 }
 
+#ifdef CONFIG_FIRMWARE_SIG
+static int firmware_sig_check(struct firmware *fw)
+{
+	struct firmware_buf *buf = fw->priv;
+	int err;
+
+	if (!buf->sig_size)
+		return -ENOENT;
+
+	err = verify_pkcs7_signature(buf->data, buf->size,
+			buf->sig_data, buf->sig_size,
+			NULL, VERIFYING_FIRMWARE_SIGNATURE, NULL, NULL);
+	if (!err)
+		buf->sig_ok = true;
+
+	return err;
+}
+#endif /* CONFIG_FIRMWARE_SIG */
+
 /*
  * user-mode helper code
  */
@@ -1224,6 +1297,83 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
 	return retval;
 }
 
+static ssize_t firmware_sig_data_read(struct file *filp, struct kobject *kobj,
+				      struct bin_attribute *bin_attr,
+				      char *buffer, loff_t offset, size_t count)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct firmware_priv *fw_priv = to_firmware_priv(dev);
+	struct firmware_buf *buf;
+	ssize_t ret_count;
+
+	mutex_lock(&fw_lock);
+	buf = fw_priv->buf;
+	if (!buf || fw_state_is_done(&buf->fw_st)) {
+		ret_count = -ENODEV;
+		goto out;
+	}
+	if (offset > buf->size) {
+		ret_count = 0;
+		goto out;
+	}
+	if (count > buf->size - offset)
+		ret_count = buf->size - offset;
+	else
+		ret_count = count;
+
+	memcpy(buffer, buf->data + offset, ret_count);
+
+out:
+	mutex_unlock(&fw_lock);
+	return ret_count;
+}
+
+static ssize_t firmware_sig_data_write(struct file *filp, struct kobject *kobj,
+				       struct bin_attribute *bin_attr,
+				       char *buffer, loff_t offset,
+				       size_t count)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct firmware_priv *fw_priv = to_firmware_priv(dev);
+	struct firmware_buf *buf;
+	void *buf_tmp;
+	size_t new_size;
+	ssize_t ret_count;
+
+	if (!capable(CAP_SYS_RAWIO))
+		return -EPERM;
+
+	mutex_lock(&fw_lock);
+	buf = fw_priv->buf;
+	if (!buf || fw_state_is_done(&buf->fw_st)) {
+		ret_count = -ENODEV;
+		goto out;
+	}
+
+	if (buf->sig_data && (offset + count > buf->sig_size)) {
+		/* grow a buffer */
+		new_size = min(size_t, PAGE_SIZE, offset + count);
+		buf_tmp = NULL;
+		if (new_size <= PAGE_SIZE)
+			buf_tmp = vmalloc(new_size);
+		if (!buf_tmp)
+			ret_count = -ENOMEM;
+			goto out;
+		}
+		memcpy(buf_tmp, buf->sig_data, buf->sig_size);
+		vfree(buf->sig_data);
+		buf->sig_data = buf_tmp;
+		buf->sig_size = new_size;
+	}
+
+	ret_count = min(size_t, buf->sig_size - offset, count);
+	memcpy(buf->data + offset, buffer, ret_count);
+
+out:
+	mutex_unlock(&fw_lock);
+	return ret_count;
+}
+
 static struct bin_attribute firmware_attr_data = {
 	.attr = { .name = "data", .mode = 0644 },
 	.size = 0,
@@ -1231,6 +1381,13 @@ static struct bin_attribute firmware_attr_data = {
 	.write = firmware_data_write,
 };
 
+static struct bin_attribute firmware_attr_sig_data = {
+	.attr = { .name = "sig_data", .mode = 0644 },
+	.size = 0,
+	.read = firmware_sig_data_read,
+	.write = firmware_sig_data_write,
+};
+
 static struct attribute *fw_dev_attrs[] = {
 	&dev_attr_loading.attr,
 	NULL
@@ -1238,6 +1395,7 @@ static struct attribute *fw_dev_attrs[] = {
 
 static struct bin_attribute *fw_dev_bin_attrs[] = {
 	&firmware_attr_data,
+	&firmware_attr_sig_data,
 	NULL
 };
 
@@ -1364,9 +1522,6 @@ static int fw_load_from_user_helper(struct firmware *firmware,
 	fw_priv->buf = firmware->priv;
 	ret = _request_firmware_load(fw_priv, data_params, timeout);
 
-	if (!ret)
-		ret = assign_firmware_buf(firmware, device, data_params);
-
 out_unlock:
 	usermodehelper_read_unlock();
 
@@ -1490,11 +1645,43 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 				 "Direct firmware load for %s failed with error %d\n",
 				 name, ret);
 		ret = driver_data_fallback(fw, name, device, data_params, ret);
-	} else
-		ret = assign_firmware_buf(fw, device, data_params);
+	}
+	if (ret)
+		goto out;
+
+#ifdef CONFIG_FIRMWARE_SIG
+	if (driver_data_param_no_sig_check(&data_params->req_params))
+		goto skip_sig_check;
+
+	/* Verify the data with its signature */
+	ret = firmware_sig_check(fw);
+	if (ret) {
+		if (fw_sig_enforce) {
+			/* ENFORCING */
+			dev_err(device,
+			"firmware [%s] verification failed (%d).\n",
+				name, ret);
+
+			goto out;
+		}
+
+		/* PERMISSIVE */
+		dev_warn(device,
+		"firmware [%s] verification failed (%d), but this is ok.\n",
+				name, ret);
+	} else {
+		dev_info(device, "firmware [%s] verification succeeded.\n",
+				name);
+	}
+
+ skip_sig_check:
+#endif /* CONFIG_FIRMWARE_SIG */
+	ret = assign_firmware_buf(fw, device, data_params);
 
  out:
 	if (ret < 0) {
+		dev_warn(device, "Loading firmware failed (%d)\n", ret);
+
 		release_firmware(fw);
 		fw = NULL;
 	}
diff --git a/include/linux/driver_data.h b/include/linux/driver_data.h
index cfeaacfd74d3..039b77b82a91 100644
--- a/include/linux/driver_data.h
+++ b/include/linux/driver_data.h
@@ -123,11 +123,16 @@ union driver_data_cbs {
  *	file to be present given the API range, it is only required for one
  *	file in the API range to be present.  If the %DRIVER_DATA_REQ_OPTIONAL
  *	flag is also enabled then all files are treated as optional.
+ * @DRIVER_DATA_REQ_NO_SIG_CHECK: indicates that signature verification
+ *	for this driver data is not required for any reason.
+ *	So even if CONFIG_FIRMWARE_SIG_ENFORCE is enabled, the data will be
+ *	successfully loaded if it does exist.
  */
 enum driver_data_reqs {
 	DRIVER_DATA_REQ_OPTIONAL			= 1 << 0,
 	DRIVER_DATA_REQ_KEEP				= 1 << 1,
 	DRIVER_DATA_REQ_USE_API_VERSIONING		= 1 << 2,
+	DRIVER_DATA_REQ_NO_SIG_CHECK			= 1 << 3,
 };
 
 /**
-- 
2.11.1

[PATCH 2/4] scripts: sign-file: add firmware-signing option

[AKASHI Takahiro]

This new option (-f) allows us to create a signature file (*.p7s) for
firmware binary.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
---
 scripts/sign-file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index fbd34b8e8f57..211f6531fd7e 100644
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -242,8 +242,11 @@ int main(int argc, char **argv)
 #endif
 
 	do {
-		opt = getopt(argc, argv, "sdpk");
+		opt = getopt(argc, argv, "fsdpk");
 		switch (opt) {
+		case 'f':
+			  use_signed_attrs = 0;
+			  sign_only = true; save_sig = true; break;
 		case 's': raw_sig = true; break;
 		case 'p': save_sig = true; break;
 		case 'd': sign_only = true; save_sig = true; break;
-- 
2.11.1

[PATCH 3/4] test: firmwware: add signature test to driver_data loader test

[AKASHI Takahiro]

Add the following tests:
No.15 - verify loaded data witha signature
No.16 - verify loaded data witha signature under signature enforcing

For each test, there are several test cases:
* no signature provided
* valid signature provided
* invalid signature provided

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
---
 lib/test_driver_data.c                          |  56 ++++-
 tools/testing/selftests/firmware/driver_data.sh | 265 +++++++++++++++++++++++-
 2 files changed, 315 insertions(+), 6 deletions(-)

diff --git a/lib/test_driver_data.c b/lib/test_driver_data.c
index 422ea6289396..660cf9ff9ac0 100644
--- a/lib/test_driver_data.c
+++ b/lib/test_driver_data.c
@@ -85,6 +85,9 @@ int num_test_devs;
  * @api_min: API min version to use for the test.
  * @api_max: API max version to use for the test.
  * @api_name_postfix: API name postfix
+ * @no_sig_check: whether or not we wish to verify the driver_data with its
+ *      signature. If CONFIG_FIRMWARE_SIG_ENFORCE, passing the verification
+ *      is mandatory.
  * @test_result: a test may use this to collect the result from the call
  *	of the driver_data_request_async() or driver_data_request_sync() calls
  *	used in their tests. Note that for async calls this typically will be a
@@ -125,6 +128,7 @@ struct test_config {
 	u8 api_min;
 	u8 api_max;
 	char *api_name_postfix;
+	bool no_sig_check;
 
 	int test_result;
 };
@@ -345,6 +349,9 @@ static ssize_t config_show(struct device *dev,
 	len += snprintf(buf+len, PAGE_SIZE,
 			"keep:\t\t%s\n",
 			config->keep ? "true" : "false");
+	len += snprintf(buf+len, PAGE_SIZE,
+			"no_sig_check:\t%s\n",
+			config->no_sig_check ? "true" : "false");
 
 	mutex_unlock(&test_dev->config_mutex);
 
@@ -443,6 +450,9 @@ static int config_sync_req_cb(void *context,
 {
 	struct driver_data_test_device *test_dev = context;
 
+	if (unused_error)
+		return unused_error;
+
 	return config_load_data(test_dev, driver_data);
 }
 
@@ -455,14 +465,19 @@ static int trigger_config_sync(struct driver_data_test_device *test_dev)
 					      (config->optional ?
 					       DRIVER_DATA_REQ_OPTIONAL : 0) |
 					      (config->keep ?
-					       DRIVER_DATA_REQ_KEEP : 0)),
+					       DRIVER_DATA_REQ_KEEP : 0) |
+					      (config->no_sig_check ?
+					       DRIVER_DATA_REQ_NO_SIG_CHECK :
+					       0)),
 	};
 	const struct driver_data_req_params req_params_opt_cb = {
 		DRIVER_DATA_DEFAULT_SYNC(config_sync_req_cb, test_dev),
 		DRIVER_DATA_SYNC_OPT_CB(config_sync_req_default_cb,
 					     test_dev),
 		.reqs = (config->optional ? DRIVER_DATA_REQ_OPTIONAL : 0) |
-			(config->keep ? DRIVER_DATA_REQ_KEEP : 0),
+			(config->keep ? DRIVER_DATA_REQ_KEEP : 0) |
+			(config->no_sig_check ? DRIVER_DATA_REQ_NO_SIG_CHECK :
+						0),
 	};
 	const struct driver_data_req_params *req_params;
 
@@ -528,20 +543,26 @@ static int trigger_config_async(struct driver_data_test_device *test_dev)
 	const struct driver_data_req_params req_params_default = {
 		DRIVER_DATA_DEFAULT_ASYNC(config_async_req_cb, test_dev),
 		.reqs = (config->optional ? DRIVER_DATA_REQ_OPTIONAL : 0) |
-			(config->keep ? DRIVER_DATA_REQ_KEEP : 0),
+			(config->keep ? DRIVER_DATA_REQ_KEEP : 0) |
+			(config->no_sig_check ? DRIVER_DATA_REQ_NO_SIG_CHECK :
+						0),
 	};
 	const struct driver_data_req_params req_params_opt_cb = {
 		DRIVER_DATA_DEFAULT_ASYNC(config_async_req_cb, test_dev),
 		DRIVER_DATA_ASYNC_OPT_CB(config_async_req_default_cb, test_dev),
 		.reqs = (config->optional ? DRIVER_DATA_REQ_OPTIONAL : 0) |
-			(config->keep ? DRIVER_DATA_REQ_KEEP : 0),
+			(config->keep ? DRIVER_DATA_REQ_KEEP : 0) |
+			(config->no_sig_check ? DRIVER_DATA_REQ_NO_SIG_CHECK :
+						0),
 	};
 	const struct driver_data_req_params req_params_api = {
 		DRIVER_DATA_API_CB(config_async_req_api_cb, test_dev),
 		DRIVER_DATA_API(config->api_min, config->api_max, config->api_name_postfix),
 		.reqs = (config->optional ? DRIVER_DATA_REQ_OPTIONAL : 0) |
 			(config->keep ? DRIVER_DATA_REQ_KEEP : 0) |
-			(config->use_api_versioning ? DRIVER_DATA_REQ_USE_API_VERSIONING : 0),
+			(config->use_api_versioning ? DRIVER_DATA_REQ_USE_API_VERSIONING : 0) |
+			(config->no_sig_check ? DRIVER_DATA_REQ_NO_SIG_CHECK :
+						0),
 	};
 	const struct driver_data_req_params *req_params;
 
@@ -670,6 +691,7 @@ static int __driver_data_config_init(struct test_config *config)
 	config->use_api_versioning = false;
 	config->api_min = 0;
 	config->api_max = 0;
+	config->no_sig_check = false;
 	config->test_result = 0;
 
 	return 0;
@@ -1108,6 +1130,29 @@ static ssize_t config_api_name_postfix_show(struct device *dev,
 static DEVICE_ATTR(config_api_name_postfix, 0644, config_api_name_postfix_show,
 		   config_api_name_postfix_store);
 
+static ssize_t config_no_sig_check_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	struct driver_data_test_device *test_dev = dev_to_test_dev(dev);
+	struct test_config *config = &test_dev->config;
+
+	return test_dev_config_update_bool(test_dev, buf, count,
+					   &config->no_sig_check);
+}
+
+static ssize_t config_no_sig_check_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct driver_data_test_device *test_dev = dev_to_test_dev(dev);
+	struct test_config *config = &test_dev->config;
+
+	return test_dev_config_show_bool(test_dev, buf, config->no_sig_check);
+}
+static DEVICE_ATTR(config_no_sig_check, 0644, config_no_sig_check_show,
+		   config_no_sig_check_store);
+
 static ssize_t test_result_store(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
@@ -1147,6 +1192,7 @@ static struct attribute *test_dev_attrs[] = {
 	TEST_DRIVER_DATA_DEV_ATTR(config_api_min),
 	TEST_DRIVER_DATA_DEV_ATTR(config_api_max),
 	TEST_DRIVER_DATA_DEV_ATTR(config_api_name_postfix),
+	TEST_DRIVER_DATA_DEV_ATTR(config_no_sig_check),
 	TEST_DRIVER_DATA_DEV_ATTR(test_result),
 
 	NULL,
diff --git a/tools/testing/selftests/firmware/driver_data.sh b/tools/testing/selftests/firmware/driver_data.sh
index bfd0436cce47..f9c52a87b9d0 100755
--- a/tools/testing/selftests/firmware/driver_data.sh
+++ b/tools/testing/selftests/firmware/driver_data.sh
@@ -54,6 +54,10 @@ ALL_TESTS="$ALL_TESTS 0013:1:1"
 # system_wakeupakeup | socat - /dev/pts/7,raw,echo=0,crnl
 #ALL_TESTS="$ALL_TESTS 0014:0:1"
 
+# Before running these tests, we must prepare an appropriate signature.
+#ALL_TESTS="$ALL_TESTS 0015:1:1"
+#ALL_TESTS="$ALL_TESTS 0016:1:1"
+
 test_modprobe()
 {
        if [ ! -d $DIR ]; then
@@ -100,6 +104,10 @@ function allow_user_defaults()
 
 	# This is an unlikely real-world firmware content. :)
 	echo "ABCD0123" >"$FW"
+
+	# TODO: some set-up is necessary in advance
+	cp ${TEST_DIR}/fwbin/*sig*-*.bin ${FWPATH}
+	cp ${TEST_DIR}/fwbin/*sig*-*.bin.p7s ${FWPATH}
 }
 
 test_reqs()
@@ -133,7 +141,7 @@ test_finish()
 {
 	echo -n "$OLD_PATH" >/sys/module/firmware_class/parameters/path
 	rm -f "$FW"
-	rmdir "$FWPATH"
+	rm -rf "$FWPATH"
 }
 
 errno_name_to_val()
@@ -147,6 +155,12 @@ errno_name_to_val()
 		echo -2;;
 	-EINVAL)
 		echo -22;;
+	-EBADMSG)
+		echo -74;;
+	-ENOKEY)
+		echo -126;;
+	-EKEYREJECTED)
+		echo -129;;
 	-ERR_ANY)
 		echo -123456;;
 	*)
@@ -164,12 +178,37 @@ errno_val_to_name()
 		echo -ENOENT;;
 	-22)
 		echo -EINVAL;;
+	-74)
+		echo -EBADMSG;;
+	-126)
+		echo -ENOKEY;;
+	-129)
+		echo -EKEYREJECTED;;
 	-123456)
 		echo -ERR_ANY;;
 	*)
 		echo invalid;;
 	esac
 
+is_not_sig_enforced()
+{
+	if [ "Y" == \
+		$(cat /sys/module/firmware_class/parameters/fw_sig_enforce) ]
+	then
+		echo "$0: Signature enforced. Cannot run this test." >&2
+		exit 1
+	fi
+}
+
+enforce_sig_check()
+{
+	if ! echo -n 1 >/sys/module/firmware_class/parameters/fw_sig_enforce
+	then
+		echo "$0: Unable to set to fw_sig_enforce" >&2
+		exit 1
+	fi
+}
+
 config_set_async()
 {
 	if ! echo -n 1 >$DIR/config_async ; then
@@ -202,6 +241,22 @@ config_disable_optional()
 	fi
 }
 
+config_set_sig_check()
+{
+	if ! echo -n 0 >$DIR/config_no_sig_check ; then
+		echo "$0: Unable to set to sig_check" >&2
+		exit 1
+	fi
+}
+
+config_disable_sig_check()
+{
+	if ! echo -n 1 >$DIR/config_no_sig_check ; then
+		echo "$0: Unable to disable sig_check" >&2
+		exit 1
+	fi
+}
+
 config_set_keep()
 {
 	if ! echo -n 1 >$DIR/config_keep; then
@@ -820,6 +875,212 @@ driver_data_test_0014()
 	driver_data_test_0014a
 }
 
+# no signature provided, with driver not requiring it
+driver_data_test_0015_1_1()
+{
+	NAME=nosig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_disable_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# good signature, with driver not requiring it
+driver_data_test_0015_1_2()
+{
+	NAME=goodsig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_disable_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# no signature provided (ENOENT)
+driver_data_test_0015_2()
+{
+	NAME=nosig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# good signature
+driver_data_test_0015_3()
+{
+	NAME=goodsig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# signature not signed with a trusted key (ENOKEY)
+driver_data_test_0015_4()
+{
+	NAME=badsig1-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# digest doens't match (wrong hash value) (EKEYREJECTED)
+driver_data_test_0015_5()
+{
+	NAME=badsig2-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# signature corrupted/parsing error (EBADMSG)
+driver_data_test_0015_6()
+{
+	NAME=badsig3-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+driver_data_test_0015()
+{
+	is_not_sig_enforced
+
+	driver_data_test_0015_1_1
+	driver_data_test_0015_1_2
+	driver_data_test_0015_2
+	driver_data_test_0015_3
+	driver_data_test_0015_4
+	driver_data_test_0015_5
+	driver_data_test_0015_6
+}
+
+# no signature provided, with driver not requiring it
+driver_data_test_0016_1_1()
+{
+	NAME=nosig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_disable_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# good signature, with driver not requiring it
+driver_data_test_0016_1_2()
+{
+	NAME=goodsig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_disable_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# no signature provided (ENOENT)
+driver_data_test_0016_2()
+{
+	NAME=nosig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger_want_fail ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} -ENOENT
+}
+
+# good signature
+driver_data_test_0016_3()
+{
+	NAME=goodsig-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger ${FUNCNAME[0]}
+	config_file_should_match ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} SUCCESS
+}
+
+# signature not signed with a trusted key (ENOKEY)
+driver_data_test_0016_4()
+{
+	NAME=badsig1-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger_want_fail ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} -ENOKEY
+}
+
+# digest doens't match (wrong hash value) (EKEYREJECTED)
+driver_data_test_0016_5()
+{
+	NAME=badsig2-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger_want_fail ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} -EKEYREJECTED
+}
+
+# signature corrupted/parsing error (EBADMSG)
+driver_data_test_0016_6()
+{
+	NAME=badsig3-$DEFAULT_DRIVER_DATA
+
+	driver_data_set_sync_defaults
+	config_set_name $NAME
+	config_set_sig_check
+	config_trigger_want_fail ${FUNCNAME[0]}
+	config_expect_result ${FUNCNAME[0]} -EBADMSG
+}
+
+# This test case must be run at the last.
+# Otherwe we must reboot the kernel to reset fw_sig_enforce.
+driver_data_test_0016()
+{
+	enforce_sig_check
+
+	driver_data_test_0016_1_1
+	driver_data_test_0016_1_2
+	driver_data_test_0016_2
+	driver_data_test_0016_3
+	driver_data_test_0016_4
+	driver_data_test_0016_5
+	driver_data_test_0016_6
+}
+
 list_tests()
 {
 	echo "Test ID list:"
@@ -842,6 +1103,8 @@ list_tests()
 	echo "0012 x $(get_test_count 0012) - Verify api call wills will hunt for files, ignore file"
 	echo "0013 x $(get_test_count 0013) - Verify api call works"
 	echo "0014 x $(get_test_count 0013) - Verify api call works with suspend + resume"
+	echo "0015 x $(get_test_count 0015) - Verify loaded data with signature"
+	echo "0016 x $(get_test_count 0016) - Verify loaded data with signature under signature enforcing"
 }
 
 test_reqs
-- 
2.11.1

[PATCH 4/4] firmware: document signature verification for driver data

[AKASHI Takahiro]

add descriptions and usage about firmware signing in driver data APIs.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
---
 Documentation/driver-api/firmware/driver_data.rst  |  6 ++
 .../driver-api/firmware/fallback-mechanisms.rst    |  5 +-
 Documentation/driver-api/firmware/signing.rst      | 81 ++++++++++++++++++++++
 3 files changed, 90 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/driver-api/firmware/signing.rst

diff --git a/Documentation/driver-api/firmware/driver_data.rst b/Documentation/driver-api/firmware/driver_data.rst
index be7c7ff99151..cdc47144a8b2 100644
--- a/Documentation/driver-api/firmware/driver_data.rst
+++ b/Documentation/driver-api/firmware/driver_data.rst
@@ -94,6 +94,12 @@ in these callbacks it frees it for you by default after callback handlers
 are issued. If you wish to keep the driver data around after your callbacks
 you must specify this through the driver data request parameter data structure.
 
+Signature verification
+======================
+
+  * `data signing`_
+.. _`data signing`: ./signing.rst
+
 Driver data private internal functionality
 ==========================================
 
diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index d19354794e67..e557d6630330 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -81,11 +81,12 @@ and a file upload firmware into:
 
   * /sys/$DEVPATH/loading
   * /sys/$DEVPATH/data
+  * /sys/$DEVPATH/sig_data
 
 To upload firmware you will echo 1 onto the loading file to indicate
 you are loading firmware. You then cat the firmware into the data file,
-and you notify the kernel the firmware is ready by echo'ing 0 onto
-the loading file.
+optionally its signature file, and you notify the kernel the firmware is
+ready by echo'ing 0 onto the loading file.
 
 The firmware device used to help load firmware using sysfs is only created if
 direct firmware loading fails and if the fallback mechanism is enabled for your
diff --git a/Documentation/driver-api/firmware/signing.rst b/Documentation/driver-api/firmware/signing.rst
new file mode 100644
index 000000000000..2dbee104700e
--- /dev/null
+++ b/Documentation/driver-api/firmware/signing.rst
@@ -0,0 +1,81 @@
+================================
+Kernel firmware signing facility
+================================
+
+Overview
+========
+
+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 `Kernel module signing`_.
+
+.. _`Kernel module signing`: /admin-guide/module-signing.rst
+
+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 the following commands:
+
+        scripts/sign-file -f sha256 \
+		$PRIVATE_KEY_FILE_IN_PEM_FORM \
+		$X509_CERT_FILE_IN_PEM_FORM \
+		$FIRMWARE_BLOB_NAME
+
+  or
+
+	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
-- 
2.11.1

Re: [PATCH 1/4] firmware: add firmware signing

[Alan Cox]

On Fri, 2017-05-26 at 12:06 +0900, AKASHI Takahiro wrote:
> There is one driver data option, DRIVER_DATA_REQ_NO_SIG_CHECK,
> which will skip signature verification check at load time
> even in enforcing mode.
> This option is solely for non security-sensitive data.

It's also for firmware that is already signed and checked by the
hardware. In the x86 world almost all modern era firmware is already
signed and the signature checked by the device.

> +static ssize_t firmware_sig_data_write(struct file *filp, struct
> kobject *kobj,
> +				       struct bin_attribute
> *bin_attr,
> +				       char *buffer, loff_t offset,
> +				       size_t count)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct firmware_priv *fw_priv = to_firmware_priv(dev);
> +	struct firmware_buf *buf;
> +	void *buf_tmp;
> +	size_t new_size;
> +	ssize_t ret_count;
> +
> +	if (!capable(CAP_SYS_RAWIO))
> +		return -EPERM;
> +
> +	mutex_lock(&fw_lock);
> +	buf = fw_priv->buf;
> +	if (!buf || fw_state_is_done(&buf->fw_st)) {
> +		ret_count = -ENODEV;
> +		goto out;
> +	}
> +
> +	if (buf->sig_data && (offset + count > buf->sig_size)) {

If I do a ridiculously long amount of I/O what stops offset + count
overflowing ? It's no big deal as its CAP_SYS_RAWIO anyway but I'm just
wondering if there is a test missing ?

Alan