Message-Id: <20170526030609.1414-2-takahiro.akashi@linaro.org>
Date:   Fri, 26 May 2017 12:06:06 +0900
From:   AKASHI Takahiro <takahiro.akashi@...aro.org>
To:     mcgrof@...nel.org
Cc:     rusty@...tcorp.com.au, dhowells@...hat.com, ming.lei@...onical.com,
        seth.forshee@...onical.com, kyle@...nel.org,
        David.Woodhouse@...el.com, linux-kernel@...r.kernel.org,
        AKASHI Takahiro <takahiro.akashi@...aro.org>,
        "Luis R . Rodriguez" <mcgrof@...e.com>
Subject: [PATCH 1/4] firmware: add firmware signing

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@...tcorp.com.au>
Cc: David Howells <dhowells@...hat.com>
Cc: Ming Lei <ming.lei@...onical.com>
Cc: Seth Forshee <seth.forshee@...onical.com>
Cc: Kyle McMartin <kyle@...nel.org>
Cc: David Woodhouse <David.Woodhouse@...el.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@...e.com>
[akashi:migrated to driver data APIs]
Signed-off-by: AKASHI Takahiro <takahiro.akashi@...aro.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

Powered by blists - more mailing lists