fw_lockdown: new micro LSM module to prevent loading unsigned firmware

Started by Mimi Zohar on Nov 10, 2017 · 7 messages from 4 authors · Last reply Apr 3, 2018

Subsystems (2)

Maintainers (4)

STALE3145d

open the first message on its own page

Message details and related content
If the kernel is locked down and IMA-appraisal is not enabled, prevent
loading of unsigned firmware.

Signed-off-by: Mimi Zohar <redacted>
---

Changelog v1:
- Lots of minor changes Kconfig, Makefile, fw_lsm.c for such a small patch

 security/Kconfig              |  1 +
 security/Makefile             |  2 ++
 security/fw_lockdown/Kconfig  |  6 +++++
 security/fw_lockdown/Makefile |  3 +++
 security/fw_lockdown/fw_lsm.c | 51 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+)
 create mode 100644 security/fw_lockdown/Kconfig
 create mode 100644 security/fw_lockdown/Makefile
 create mode 100644 security/fw_lockdown/fw_lsm.c
diff --git a/security/Kconfig b/security/Kconfig
index a4fa8b826039..6e7e5888f823 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -243,6 +243,7 @@ source security/tomoyo/Kconfig
 source security/apparmor/Kconfig
 source security/loadpin/Kconfig
 source security/yama/Kconfig
+source security/fw_lockdown/Kconfig
 
 source security/integrity/Kconfig
 
diff --git a/security/Makefile b/security/Makefile
index 8c4a43e3d4e0..58852dee5e22 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -9,6 +9,7 @@ subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
 subdir-$(CONFIG_SECURITY_APPARMOR)	+= apparmor
 subdir-$(CONFIG_SECURITY_YAMA)		+= yama
 subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
+subdir-$(CONFIG_SECURITY_FW_LOCKDOWN)	+= fw_lockdown
 
 # always enable default capabilities
 obj-y					+= commoncap.o
@@ -24,6 +25,7 @@ obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
 obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
 obj-$(CONFIG_SECURITY_YAMA)		+= yama/
 obj-$(CONFIG_SECURITY_LOADPIN)		+= loadpin/
+obj-$(CONFIG_SECURITY_FW_LOCKDOWN)	+= fw_lockdown/
 obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
 
 # Object integrity file lists
diff --git a/security/fw_lockdown/Kconfig b/security/fw_lockdown/Kconfig
new file mode 100644
index 000000000000..d6aef6ce8fee
--- /dev/null
+++ b/security/fw_lockdown/Kconfig
@@ -0,0 +1,6 @@
+config SECURITY_FW_LOCKDOWN
+	bool "Prevent loading unsigned firmware"
+	depends on LOCK_DOWN_KERNEL
+	default y
+	help
+	  Prevent loading unsigned firmware in lockdown mode,
diff --git a/security/fw_lockdown/Makefile b/security/fw_lockdown/Makefile
new file mode 100644
index 000000000000..3a16757fd35d
--- /dev/null
+++ b/security/fw_lockdown/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SECURITY_FW_LOCKDOWN) += fw_lockdown.o
+
+fw_lockdown-y := fw_lsm.o
diff --git a/security/fw_lockdown/fw_lsm.c b/security/fw_lockdown/fw_lsm.c
new file mode 100644
index 000000000000..cce03a5c5280
--- /dev/null
+++ b/security/fw_lockdown/fw_lsm.c
@@ -0,0 +1,51 @@
+/*
+ * fw_lockdown security module
+ *
+ * Copyright (C) 2017 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "fw_lockdown: " fmt
+
+#include <linux/module.h>
+#include <linux/ima.h>
+#include <linux/lsm_hooks.h>
+
+/**
+ * fw_lockdown_read_file - prevent loading of unsigned firmware
+ * @file: pointer to firmware
+ * @read_id: caller identifier
+ *
+ * Prevent loading of unsigned firmware in lockdown mode.
+ */
+static int fw_lockdown_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	if (id == READING_FIRMWARE) {
+		if (!is_ima_appraise_enabled() &&
+		    !kernel_is_locked_down("Loading of unsigned firmware"))
+			return -EACCES;
+	}
+	return 0;
+}
+
+static struct security_hook_list fw_lockdown_hooks[] = {
+	LSM_HOOK_INIT(kernel_read_file, fw_lockdown_read_file)
+};
+
+static int __init init_fw_lockdown(void)
+{
+	security_add_hooks(fw_lockdown_hooks, ARRAY_SIZE(fw_lockdown_hooks),
+			   "fw_lockdown");
+	pr_info("initialized\n");
+	return 0;
+}
+
+late_initcall(init_fw_lockdown);
+MODULE_LICENSE("GPL");
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Message details and related content
On Fri, Nov 10, 2017 at 04:02:55PM -0500, Mimi Zohar wrote:
If the kernel is locked down and IMA-appraisal is not enabled, prevent
loading of unsigned firmware.

Signed-off-by: Mimi Zohar <redacted>
---

Changelog v1:
- Lots of minor changes Kconfig, Makefile, fw_lsm.c for such a small patch
<-- snip -->
quoted hunk
diff --git a/security/fw_lockdown/fw_lsm.c b/security/fw_lockdown/fw_lsm.c
new file mode 100644
index 000000000000..cce03a5c5280
--- /dev/null
+++ b/security/fw_lockdown/fw_lsm.c
@@ -0,0 +1,51 @@
+/*
+ * fw_lockdown security module
+ *
+ * Copyright (C) 2017 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "fw_lockdown: " fmt
+
+#include <linux/module.h>
+#include <linux/ima.h>
+#include <linux/lsm_hooks.h>
+
+/**
+ * fw_lockdown_read_file - prevent loading of unsigned firmware
+ * @file: pointer to firmware
+ * @read_id: caller identifier
+ *
+ * Prevent loading of unsigned firmware in lockdown mode.
+ */
+static int fw_lockdown_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	if (id == READING_FIRMWARE) {
+		if (!is_ima_appraise_enabled() &&
+		    !kernel_is_locked_down("Loading of unsigned firmware"))
+			return -EACCES;
I don't have kernel_is_locked_down() but I found it here:

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=keys-lockdown&id=0ff5adf1e81135c4ed914d0a8b70124caed04142

1) I don't see it taking any arguments
2) Don't we want:

	if (!is_ima_appraise_enabled() && kernel_is_locked_down())
		return -EACCES;

If, only if IMA appraisal is enabled would we enable kernel lockdown.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Message details and related content
On 11/10/2017 1:02 PM, Mimi Zohar wrote:
quoted hunk
If the kernel is locked down and IMA-appraisal is not enabled, prevent
loading of unsigned firmware.

Signed-off-by: Mimi Zohar <redacted>
---

Changelog v1:
- Lots of minor changes Kconfig, Makefile, fw_lsm.c for such a small patch

 security/Kconfig              |  1 +
 security/Makefile             |  2 ++
 security/fw_lockdown/Kconfig  |  6 +++++
 security/fw_lockdown/Makefile |  3 +++
 security/fw_lockdown/fw_lsm.c | 51 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+)
 create mode 100644 security/fw_lockdown/Kconfig
 create mode 100644 security/fw_lockdown/Makefile
 create mode 100644 security/fw_lockdown/fw_lsm.c
diff --git a/security/Kconfig b/security/Kconfig
index a4fa8b826039..6e7e5888f823 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -243,6 +243,7 @@ source security/tomoyo/Kconfig
 source security/apparmor/Kconfig
 source security/loadpin/Kconfig
 source security/yama/Kconfig
+source security/fw_lockdown/Kconfig
 
 source security/integrity/Kconfig
 
diff --git a/security/Makefile b/security/Makefile
index 8c4a43e3d4e0..58852dee5e22 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -9,6 +9,7 @@ subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
 subdir-$(CONFIG_SECURITY_APPARMOR)	+= apparmor
 subdir-$(CONFIG_SECURITY_YAMA)		+= yama
 subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
+subdir-$(CONFIG_SECURITY_FW_LOCKDOWN)	+= fw_lockdown

83 more quoted lines

SECURITY_NAME_MAX is 10. Either pick a shorter name or increase
this value. I slightly favor an increase to 16.
quoted hunk
+	pr_info("initialized\n");
+	return 0;
+}
+
+late_initcall(init_fw_lockdown);
+MODULE_LICENSE("GPL");
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Message details and related content
On Fri, 2017-11-10 at 23:39 +0100, Luis R. Rodriguez wrote:
On Fri, Nov 10, 2017 at 04:02:55PM -0500, Mimi Zohar wrote:
quoted
If the kernel is locked down and IMA-appraisal is not enabled, prevent
loading of unsigned firmware.

Signed-off-by: Mimi Zohar <redacted>
---

Changelog v1:
- Lots of minor changes Kconfig, Makefile, fw_lsm.c for such a small patch
<-- snip -->
quoted
diff --git a/security/fw_lockdown/fw_lsm.c b/security/fw_lockdown/fw_lsm.c
new file mode 100644
index 000000000000..cce03a5c5280
--- /dev/null
+++ b/security/fw_lockdown/fw_lsm.c
@@ -0,0 +1,51 @@
+/*
+ * fw_lockdown security module
+ *
+ * Copyright (C) 2017 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <zohar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "fw_lockdown: " fmt
+
+#include <linux/module.h>
+#include <linux/ima.h>
+#include <linux/lsm_hooks.h>
+
+/**
+ * fw_lockdown_read_file - prevent loading of unsigned firmware

16 more quoted lines

1) I don't see it taking any arguments
David reposted the patches?https://lkml.org/lkml/2017/11/9/659?with
the change. ?The latest version of the patches are in his?efi-lock-
down branch.
2) Don't we want:

	if (!is_ima_appraise_enabled() && kernel_is_locked_down())
		return -EACCES;

If, only if IMA appraisal is enabled would we enable kernel lockdown.
Yes, the kernel_is_locked_down() test needs to be inverted.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Message details and related content
On 11/10/2017 01:02 PM, Mimi Zohar wrote:
If the kernel is locked down and IMA-appraisal is not enabled, prevent
loading of unsigned firmware.
quoted hunk
diff --git a/security/fw_lockdown/Kconfig b/security/fw_lockdown/Kconfig
new file mode 100644
index 000000000000..d6aef6ce8fee
--- /dev/null
+++ b/security/fw_lockdown/Kconfig
@@ -0,0 +1,6 @@
+config SECURITY_FW_LOCKDOWN
+	bool "Prevent loading unsigned firmware"
+	depends on LOCK_DOWN_KERNEL
+	default y
+	help
+	  Prevent loading unsigned firmware in lockdown mode,
Please be honest about what this does.  This option makes your system 
useless if you don't use IMA-Appraisal and it offers a particular 
security benefit if you do you IMA-Appraisal.  How about making it 
depend on IMA-Appraisal?  Change the name to 
SECURITY_ONLY_LOAD_IMA_APPRAISED_FIRMWARE and adjust the text 
accordingly, please.
quoted hunk
+/**
+ * fw_lockdown_read_file - prevent loading of unsigned firmware
+ * @file: pointer to firmware
+ * @read_id: caller identifier
+ *
+ * Prevent loading of unsigned firmware in lockdown mode.
That comment gives a highly misleading impression of what this function 
does.
+ */
+static int fw_lockdown_read_file(struct file *file, enum kernel_read_file_id id)
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Message details and related content
On Mon, Apr 02, 2018 at 05:42:22PM -0700, Andy Lutomirski wrote:
On 11/10/2017 01:02 PM, Mimi Zohar wrote:
quoted
If the kernel is locked down and IMA-appraisal is not enabled, prevent
loading of unsigned firmware.
quoted
diff --git a/security/fw_lockdown/Kconfig b/security/fw_lockdown/Kconfig
new file mode 100644
index 000000000000..d6aef6ce8fee
--- /dev/null
+++ b/security/fw_lockdown/Kconfig
@@ -0,0 +1,6 @@
+config SECURITY_FW_LOCKDOWN
+	bool "Prevent loading unsigned firmware"
+	depends on LOCK_DOWN_KERNEL
+	default y
+	help
+	  Prevent loading unsigned firmware in lockdown mode,
Please be honest about what this does.  This option makes your system
useless if you don't use IMA-Appraisal and it offers a particular security
benefit if you do you IMA-Appraisal.  How about making it depend on
IMA-Appraisal?  Change the name to SECURITY_ONLY_LOAD_IMA_APPRAISED_FIRMWARE
and adjust the text accordingly, please.
Mimi is on vacation right now so I'll address this.

All the above makes sense, but just keep in mind the patch was posted just as
an illustration of how IMA *can* live up to the original intent proposed by
David Howells on kernel lockdown. David's old kernel lockdown documentation
stipulated that a requirement was to also prevent loading unsigned firmware.

Does the kernel lockdown documentation still have a section indicating signed
firmware is a requirement? Or was that removed in the end?

Mimi's RFC was at a time when we still had on our roadmap the prospect of
adding generic signed firmware support into Linux. Mimi's point and RFC patch
was an illustration then on how IMA users can already meet these assumed
requirements on 'kernel lockdown', and that if we used a micro LSM hook this
could be easily managed, given the firmware signing support we intended on
adding would not be needed for IMA systems. Her point was that with IMA you can
already meet the definition and IMA systems would not have to wait for "generic
firmware signing" to be merged.

The biggest thing which has changed since then is that we decided to *not*
support or streamline generic firmware signing (non-IMA) for now for a few
reasons [0] [1] which are important to re-iterate as these are easy to forget,
and AFAICT not documented anywhere.

One was that my own requirement for it was already done -- cfg80211 already
open codes firmware signing checking on its own through the new
CONFIG_CFG80211_REQUIRE_SIGNED_REGDB thereby also not requiring CRDA anymore,
the userspace component which used to read the signed regulatory database and
then send regulatory content to the wireless subsystem. So CRDA is no longer
needed if you enable CONFIG_CFG80211_REQUIRE_SIGNED_REGDB.

Second, is is that the whole UEFI movement pushed hardware vendors to start
embracing requiring signed firmware on peripherals themselves. So a lot of
hardware these days *does* do firmware signing already and a generic kernel
firmware signing facility would then just make us do double work then. There
are exceptions and there are a few reasons for this. For instance USB devices
are not allowed to pee on random memory (thanks for the analogy Alan!), so
not all USB devices have support for checking their own firmware signature.
This is one reason also why why some operating systems do not allow users

56 more lines

quoted
+/**
+ * fw_lockdown_read_file - prevent loading of unsigned firmware
+ * @file: pointer to firmware
+ * @read_id: caller identifier
+ *
+ * Prevent loading of unsigned firmware in lockdown mode.
That comment gives a highly misleading impression of what this function
does.
quoted
+ */
+static int fw_lockdown_read_file(struct file *file, enum kernel_read_file_id id)
-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Message details and related content
On Tue, Apr 3, 2018 at 9:56 AM, Luis R. Rodriguez [off-list ref] wrote:
The biggest thing which has changed since then is that we decided to *not*
support or streamline generic firmware signing (non-IMA) for now for a few
reasons [0] [1] which are important to re-iterate as these are easy to forget,
and AFAICT not documented anywhere.
And the URLs...

[0] https://lkml.kernel.org/r/20171204195155.GU729 at wotan.suse.de
[1] https://lkml.kernel.org/r/20171207153209.5da771a9 at alans-desktop

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help