Skip to contentSkip to conversation

fw_lockdown: new micro LSM module to prevent loading unsigned firmware

RFC PATCH In review Mimi Zohar 3 participants 10 messages Nov 13, 2017 – Nov 23, 2017 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

Mimi Zohar

author
#0 lore raw

RFC PATCH v2 1/1 fw_lockdown: new micro LSM module to prevent loading unsigned firmware

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

---
Changelog v2:
- Invert kernel_is_locked_down() test (Luis Rodriquez)
- Increase LSM name maximum size (15 bytes + null) (Casey)

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 +++++++++++++++++++++++++++++++++++++++++++
 security/security.c           |  2 +-
 6 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 security/fw_lockdown/Kconfig
 create mode 100644 security/fw_lockdown/Makefile
 create mode 100644 security/fw_lockdown/fw_lsm.c
security/Kconfig +1 -0
index a4fa8b826039..6e7e5888f823 100644
@@ -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
 
security/Makefile +2 -0
index 8c4a43e3d4e0..58852dee5e22 100644
@@ -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
security/fw_lockdown/Kconfig new file +6 -0
new file mode 100644
index 000000000000..d6aef6ce8fee
@@ -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,
security/fw_lockdown/Makefile new file +3 -0
new file mode 100644
index 000000000000..3a16757fd35d
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SECURITY_FW_LOCKDOWN) += fw_lockdown.o
+
+fw_lockdown-y := fw_lsm.o
security/fw_lockdown/fw_lsm.c new file +51 -0

51 lines not on this page · raw message

security/security.c +1 -1
index 4bf0f571b4ef..61a0c95ec687 100644
@@ -32,7 +32,7 @@
 #define MAX_LSM_EVM_XATTR	2
 
 /* Maximum number of letters for an LSM name string */
-#define SECURITY_NAME_MAX	10
+#define SECURITY_NAME_MAX	15
 
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
Signature · 1 line
2.7.4
#1 lore raw
Quoted 61 lines · Mimi Zohar

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

---
Changelog v2:
- Invert kernel_is_locked_down() test (Luis Rodriquez)
- Increase LSM name maximum size (15 bytes + null) (Casey)

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 +++++++++++++++++++++++++++++++++++++++++++
 security/security.c           |  2 +-
 6 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 security/fw_lockdown/Kconfig
 create mode 100644 security/fw_lockdown/Makefile
 create mode 100644 security/fw_lockdown/fw_lsm.c
security/Kconfig +1 -0
index a4fa8b826039..6e7e5888f823 100644
@@ -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
 
security/Makefile +2 -0
index 8c4a43e3d4e0..58852dee5e22 100644
@@ -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
security/fw_lockdown/Kconfig new file +0 -0
new file mode 100644
index 000000000000..d6aef6ce8fee

security/fw_lockdown/Kconfig line 3

Quoted 3 lines
security/fw_lockdown/Kconfig new file +3 -0
@@ -0,0 +1,6 @@
+config SECURITY_FW_LOCKDOWN
+	bool "Prevent loading unsigned firmware"
+	depends on LOCK_DOWN_KERNEL

For now depending on LOCK_DOWN_KERNEL makes sense given we have no alternative default system policy. If hashing or a default key is used later for linux-firmware, those alternatives could be added. I sprinkled some psuedo code of what I mean below.

Quoted 42 lines
security/fw_lockdown/Kconfig new file +3 -0
+	default y
+	help
+	  Prevent loading unsigned firmware in lockdown mode,
security/fw_lockdown/Makefile new file +3 -0
new file mode 100644
index 000000000000..3a16757fd35d
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SECURITY_FW_LOCKDOWN) += fw_lockdown.o
+
+fw_lockdown-y := fw_lsm.o
security/fw_lockdown/fw_lsm.c new file +24 -0
new file mode 100644
index 000000000000..9a5472bc733f
@@ -0,0 +1,51 @@
+/*
+ * fw_lockdown security module
+ *
+ * Copyright (C) 2017 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ *
+ * 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

security/fw_lockdown/fw_lsm.c line 34

Quoted 10 lines
security/fw_lockdown/fw_lsm.c new file +10 -0
+ *
+ * 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;
+	}

How about just if (id != READING_FIRMWARE) return 0 right away so that the real code of focus is not always indented. This could let the code grow nicely.

What I meant above is later we may extend this with:

if hash_available()
if !valid_hash()
return -EACCES
else if default_fw_key_available()

	if !fw_signed_default_key()
		return -EACCES;

That could be the way we support a default system policy for firmware signing, and it would not require any modifications to any firmware API callers.

Notice though that if we later want to extend support for custom requirements the semantics behind kernel_read_file() would not suffice to LSMify them, as such I'd think we'd need another call which lets the security requirements be passed.

Its unclear if IMA may want to ignore that criteria, as it does the checks in userspace. If it *can* make use of it, it could do the check-in kernel, of course.

Quoted 22 lines
security/fw_lockdown/fw_lsm.c new file +17 -0
+	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");
security/security.c +0 -0
index 4bf0f571b4ef..61a0c95ec687 100644

security/security.c line 35

Quoted 5 lines
security/security.c +1 -1
@@ -32,7 +32,7 @@
 #define MAX_LSM_EVM_XATTR	2
 
 /* Maximum number of letters for an LSM name string */
-#define SECURITY_NAME_MAX	10
+#define SECURITY_NAME_MAX	15

Should this small hunk be a separate atomic patch?

Luis

Quoted 7 lines
security/security.c +0 -0
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
Signature · 1 line
2.7.4
Signature · 2 lines
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg
author
#2 lore raw
Quoted 18 lines · Luis R. Rodriguez

security/fw_lockdown/fw_lsm.c line 34

Quoted 13 lines · Mimi Zohar

+ * 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.
+ */

security/fw_lockdown/fw_lsm.c new file +7 -0
+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;
+	}

How about just if (id != READING_FIRMWARE) return 0 right away so that the real code of focus is not always indented.

Sure

Quoted 16 lines

This could let the code
grow nicely.

What I meant above is later we may extend this with:

if hash_available()
if !valid_hash()
return -EACCES
else if default_fw_key_available()

	if !fw_signed_default_key()
		return -EACCES;

That could be the way we support a default system policy for firmware signing, and it would not require any modifications to any firmware API callers.

Quoted 7 lines

Notice though that if we later want to extend support for custom requirements the semantics behind kernel_read_file() would not suffice to LSMify them, as such I'd think we'd need another call which lets the security requirements be passed.

Its unclear if IMA may want to ignore that criteria, as it does the checks in userspace.

Huh, I kind of lost you here.  What does "it" refer to in the above sentence?  IMA is in the kernel.  So, who does what checks in userspace?

Quoted 27 lines

If it *can* make use of it, it could do the check-in kernel, of
course.

Quoted 24 lines
security/fw_lockdown/fw_lsm.c new file +17 -0
+	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");
security/security.c +0 -0
index 4bf0f571b4ef..61a0c95ec687 100644
@@ -32,7 +32,7 @@
 #define MAX_LSM_EVM_XATTR	2
 
Quoted 5 lines

security/security.c line 35

Quoted 3 lines
security/security.c +1 -1
 /* Maximum number of letters for an LSM name string */
-#define SECURITY_NAME_MAX	10
+#define SECURITY_NAME_MAX	15

Should this small hunk be a separate atomic patch?

I thought about it, but this is the first and only LSM with a larger name.

Mimi

#3 lore raw
Quoted 35 lines · Mimi Zohar
Quoted 34 lines · Luis R. Rodriguez

security/fw_lockdown/fw_lsm.c line 34

Quoted 13 lines

+ * 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.
+ */

security/fw_lockdown/fw_lsm.c new file +7 -0
+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;
+	}

This could let the code
grow nicely.

What I meant above is later we may extend this with:

if hash_available()
if !valid_hash()
return -EACCES
else if default_fw_key_available()

	if !fw_signed_default_key()
		return -EACCES;

That could be the way we support a default system policy for firmware signing, and it would not require any modifications to any firmware API callers.

Notice though that if we later want to extend support for custom requirements the semantics behind kernel_read_file() would not suffice to LSMify them, as such I'd think we'd need another call which lets the security requirements be passed.

Quoted 6 lines
Quoted 2 lines

Its unclear if IMA may want to ignore that criteria, as it does the checks in userspace.

Huh, I kind of lost you here. �What does "it" refer to in the above sentence? �IMA is in the kernel. �So, who does what checks in userspace?

Sorry I thought some checks were done in userspace, given that is clarified, what I meant is that say a device driver has a signing specification written out in the driver, should/can IMA use that on the LSM to verify the detached signature file for the firmware?

If it can be all done in kernel, it has me wondering if perhaps one option for IMA might be to do only vetting for these types of checks, where the info and description to appraise files is all in-kernel. IMA would not be required for other files.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
security/security.c +0 -0
@@ -32,7 +32,7 @@
 #define MAX_LSM_EVM_XATTR	2
 
Quoted 8 lines
Quoted 5 lines

security/security.c line 35

Quoted 3 lines
security/security.c +1 -1
 /* Maximum number of letters for an LSM name string */
-#define SECURITY_NAME_MAX	10
+#define SECURITY_NAME_MAX	15

Should this small hunk be a separate atomic patch?

I thought about it, but this is the first and only LSM with a larger name.

Maybe the commit log should mention that then.

Luis

author
#4 lore raw
Quoted 10 lines · Luis R. Rodriguez
Quoted 3 lines · Mimi Zohar

Huh, I kind of lost you here.  What does "it" refer to in the above sentence?  IMA is in the kernel.  So, who does what checks in userspace?

Sorry I thought some checks were done in userspace, given that is clarified, what I meant is that say a device driver has a signing specification written out in the driver, should/can IMA use that on the LSM to verify the detached signature file for the firmware?

IMA-appraisal currently supports file signatures as extended attributes.  Thiago Bauermann posted patches for including appended signature support to IMA-appraisal.  If someone is interested in adding detached signature support, they're welcome to do so.

Quoted 4 lines

If it can be all done in kernel, it has me wondering if perhaps one option for IMA might be to do only vetting for these types of checks, where the info and description to appraise files is all in-kernel. IMA would not be required for other files.

We probably can defer this discussion until it is applicable.

Mimi

#5 lore raw
Quoted 12 lines · Mimi Zohar
Quoted 10 lines · Luis R. Rodriguez
Quoted 3 lines · Mimi Zohar

Huh, I kind of lost you here. �What does "it" refer to in the above sentence? �IMA is in the kernel. �So, who does what checks in userspace?

Sorry I thought some checks were done in userspace, given that is clarified, what I meant is that say a device driver has a signing specification written out in the driver, should/can IMA use that on the LSM to verify the detached signature file for the firmware?

Quoted 4 lines

IMA-appraisal currently supports file signatures as extended attributes. �Thiago Bauermann posted patches for including appended signature support to IMA-appraisal. �If someone is interested in adding detached signature support, they're welcome to do so.

Neat.

Quoted 6 lines
Quoted 4 lines

If it can be all done in kernel, it has me wondering if perhaps one option for IMA might be to do only vetting for these types of checks, where the info and description to appraise files is all in-kernel. IMA would not be required for other files.

We probably can defer this discussion until it is applicable.

Fair enough :)

Luis

#6 lore raw
In reply to Luis R. Rodriguez
Quoted 9 lines · Luis R. Rodriguez
Quoted 7 lines
Quoted 4 lines

security/security.c line 35

Quoted 2 lines
security/security.c +1 -1
-#define SECURITY_NAME_MAX	10
+#define SECURITY_NAME_MAX	15

Should this small hunk be a separate atomic patch?

I thought about it, but this is the first and only LSM with a larger name.

Maybe the commit log should mention that then.

Actually, make it a separate patch, so we can easily pinpoint the commit.

Signature · 2 lines
James Morris
<[email protected]>
#7 lore raw
In reply to Luis R. Rodriguez
Quoted 15 lines · Luis R. Rodriguez
Quoted 14 lines · Mimi Zohar
Quoted 5 lines · Luis R. Rodriguez
Quoted 5 lines

+ * 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.

Quoted 8 lines

security/fw_lockdown/fw_lsm.c line 34

Quoted 8 lines

+ */

security/fw_lockdown/fw_lsm.c new file +7 -0
+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;
+	}

We also have READING_FIRMWARE_PREALLOC_BUFFER now. So the above is missing a check for that as well.

Luis

A message that is not in the archive (sent off-list or not yet received): [email protected]

#9 lore raw

Re: [Fwd: [RFC PATCH v2] fw_lockdown: new micro LSM module to prevent loading unsigned firmware]

+ a few folks who might care + lkml.

Quoted 8 lines · Mimi Zohar

Hi Matthew, David, Luis,

After Linus' comments I'm just not sure if it makes sense to re-post the patch.  To address Linus' comments, I could update the patch description as follows:

Even if firmware comes signed from the manufacturer, the signature would be verified by the HW, not the kernel.

The concept that UEFI "kernel lockdown" implies that hardware requires functionality on all or some peripherals which verifies firmware and mandates that firmware is provided in a form which is signed was *news* to me. I'd like to see this requirement carefully referenced, and think it would be good for us then to *later* extend the documentation which David Howells is working on for "kernel lockdown" to refer to these references.

Can someone provide a reference?

That said, even with this done, there are two circles of types of uses of the firmware API that this UEFI requirement does not cover:

1) platforms which do not require UEFI, and some which *may* not require it but perhaps want firmware signing. If such platforms are platforms which Red Hat or SUSE care for, then the idea of signed firmware becomes a bit more sensible for users of those platforms which may want also signed modules. If such folks exist, they need to speak up. Otherwise I'd have to say we can live UEFI as a compromise but also with IMA as a great alternative. Ie, we can shove IMA down their throats.

I've frankly have grown tired of pushing firmware signing just for the sake of the fact that I needed it for cfg80211, but now that its out of the way and we open coded it, its no longer a requirement on my part. Folks who really need this need to speak up or hold their piece for now, or just open code the signing then for areas they need it, just as cfg80211 did for regulatory data, or use IMA :)

I'll note that this UEFI requirement onto peripheral firmware signing *may* mean that non-signed firmware, and the inherent latencies built-in to revocation (or lack of such functionality) may be a detriment long term to security and obviously software freedom, but this is not a new issue.

Revocation policies should be documented on the lock down documentation, I didn't see that. Do have that well covered? Do we have revocation stuff properly handled?

Not trusting the built-in keys on UEFI was an option, does that same knob turn off this requirement on peripheral firmware verification?

2) There are uses for the firmware API for non-firmware, ie for data which hardware won't use or cannot. The cfg80211 use for regulatory data was one example, other uses are for EEPROM overrides and I'm certain there are others. I'd say just open code those for now. cfg80211 already did it.

If we grow a list of users that do the same thing then maybe we can talk about an API later. For now it doesn't make sense to me anymore given all the above.

Quoted 4 lines

  In some environments
we might want to limit providing firmware, even firmware signed by the manufacturer, to a specific instance.  Without IMA-appraisal enabled, this is not possible.

I think if you get what I put up above with references and indicate that platforms / users that wish for similar enforcement can use IMA, that it would be a good sell and alternative to UEFI + its theoretical implications on peripheral hw verification.

Quoted 6 lines

If the kernel is locked down and IMA-appraisal is not enabled, prevent loading of firmware.

Please let me know if you want me to re-post the patch with the two suggested changes and the above patch description.

Given all the above I think your patch makes sense still -- but IMHO the id tag READING_FIRMWARE_PREALLOC_BUFFER should be considered as well *first*, and I think one way forward is to evaluate removing it. The patch that added it was not properly reviewed by stakeholders and last I looked while cleaning up this firmware crap just recently [0] I could not identify the need for it [0].

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20171117-firmware-flexible
Note: use commit aaec92fe3 ("test_firmware: test the 3 firmware kernel configs using debugfs") as base for development, these patches were all posted for consideration
early for v4.16 now.

Luis

Quoted 166 lines

thanks,

Mimi

-------- Forwarded Message --------
From: Mimi Zohar <[email protected]>
To: David Howells <[email protected]>
Cc: linux-integrity <[email protected]>, linux-fsdevel < [email protected]>, linux-kernel <[email protected] .org>, Luis R. Rodriguez <[email protected]>, "AKASHI, Takahiro" <takahi [email protected]>
Subject: [RFC PATCH v2] fw_lockdown: new micro LSM module to prevent loading unsigned firmware
Date: Mon, 13 Nov 2017 06:43:34 -0500

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

---
Changelog v2:
- Invert kernel_is_locked_down() test (Luis Rodriquez)
- Increase LSM name maximum size (15 bytes + null) (Casey)

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 +++++++++++++++++++++++++++++++++++++++++++
 security/security.c           |  2 +-
 6 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 security/fw_lockdown/Kconfig
 create mode 100644 security/fw_lockdown/Makefile
 create mode 100644 security/fw_lockdown/fw_lsm.c
security/Kconfig +1 -0
index a4fa8b826039..6e7e5888f823 100644
@@ -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
 
security/Makefile +2 -0
index 8c4a43e3d4e0..58852dee5e22 100644
@@ -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
security/fw_lockdown/Kconfig new file +6 -0
new file mode 100644
index 000000000000..d6aef6ce8fee
@@ -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,
security/fw_lockdown/Makefile new file +3 -0
new file mode 100644
index 000000000000..3a16757fd35d
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SECURITY_FW_LOCKDOWN) += fw_lockdown.o
+
+fw_lockdown-y := fw_lsm.o
security/fw_lockdown/fw_lsm.c new file +51 -0
new file mode 100644
index 000000000000..9a5472bc733f
@@ -0,0 +1,51 @@
+/*
+ * fw_lockdown security module
+ *
+ * Copyright (C) 2017 IBM Corporation
+ *
+ * Authors:
+ * Mimi Zohar <[email protected]>
+ *
+ * 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");
security/security.c +1 -1
index 4bf0f571b4ef..61a0c95ec687 100644
@@ -32,7 +32,7 @@
 #define MAX_LSM_EVM_XATTR	2
 
 /* Maximum number of letters for an LSM name string */
-#define SECURITY_NAME_MAX	10
+#define SECURITY_NAME_MAX	15
 
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
Signature · 1 line
2.7.4
Signature · 2 lines
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg
author
#10 lore raw
Quoted 3 lines · Luis R. Rodriguez

I've frankly have grown tired of pushing firmware signing just for the sake of the fact that I needed it for cfg80211, but now that its out of the way and we open coded it, its no longer a requirement on my part.

As the keys CFG80211_REQUIRE_SIGNED_REGDB are built into the kernel image, they would be included in the kernel image signature.

As I previously asked?https://lkml.org/lkml/2017/11/15/679, how are the keys located in the CFG80211_EXTRA_REGDB_KEYDIR keyring trusted? ?The keyring does not validate the certificate signatures, before loading the keys on the firmware keyring. ?It explicitly bypasses the certificate signature validation.

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