[3/4] ima: add support for KEXEC_ORIG_KERNEL_CHECK
diff mbox series

Message ID 20180725233200.761-4-erichte@linux.vnet.ibm.com
State New
Headers show
Series
  • Add support for architecture-specific IMA policies
Related show

Commit Message

Eric Richter July 25, 2018, 11:31 p.m. UTC
IMA can verify the signature of kernel images loaded with kexec_file_load,
but can not verify images loaded with the regular kexec_load syscall.
Therefore, the appraisal will automatically fail during kexec_load when an
appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used
to effectively disable the kexec_load syscall, while still allowing the
kexec_file_load to operate so long as the target kernel image is signed.

However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is
enabled and there is an appraise rule set, then the target kernel would
have to be verifiable by both IMA and the architecture specific kernel
verification procedure.

This patch adds a new func= for IMA appraisal specifically for the original
kexec_load syscall. Therefore, the kexec_load syscall can be effectively
disabled via IMA policy, leaving the kexec_file_load syscall able to do its
own signature verification, and not require it to be signed via IMA. To
retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is
unchanged, and thus enables appraisal for both kexec syscalls.

Signed-off-by: Eric Richter <erichte@linux.vnet.ibm.com>
---
 Documentation/ABI/testing/ima_policy | 1 +
 security/integrity/ima/ima.h         | 2 ++
 security/integrity/ima/ima_main.c    | 3 ++-
 security/integrity/ima/ima_policy.c  | 5 +++++
 4 files changed, 10 insertions(+), 1 deletion(-)

Comments

Seth Forshee Aug. 3, 2018, 1:11 p.m. UTC | #1
On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote:
> IMA can verify the signature of kernel images loaded with kexec_file_load,
> but can not verify images loaded with the regular kexec_load syscall.
> Therefore, the appraisal will automatically fail during kexec_load when an
> appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used
> to effectively disable the kexec_load syscall, while still allowing the
> kexec_file_load to operate so long as the target kernel image is signed.
> 
> However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is
> enabled and there is an appraise rule set, then the target kernel would
> have to be verifiable by both IMA and the architecture specific kernel
> verification procedure.
> 
> This patch adds a new func= for IMA appraisal specifically for the original
> kexec_load syscall. Therefore, the kexec_load syscall can be effectively
> disabled via IMA policy, leaving the kexec_file_load syscall able to do its
> own signature verification, and not require it to be signed via IMA. To
> retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is
> unchanged, and thus enables appraisal for both kexec syscalls.

This seems like a roundabout way to disallow the kexec_load syscall.
Wouldn't it make more sense to simply disallow kexec_load any time
CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that
option impotent? Or has that idea already been rejected?

Thanks,
Seth
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 3, 2018, 2:54 p.m. UTC | #2
On Fri, 2018-08-03 at 08:11 -0500, Seth Forshee wrote:
> On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote:
> > IMA can verify the signature of kernel images loaded with kexec_file_load,
> > but can not verify images loaded with the regular kexec_load syscall.
> > Therefore, the appraisal will automatically fail during kexec_load when an
> > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used
> > to effectively disable the kexec_load syscall, while still allowing the
> > kexec_file_load to operate so long as the target kernel image is signed.
> > 
> > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is
> > enabled and there is an appraise rule set, then the target kernel would
> > have to be verifiable by both IMA and the architecture specific kernel
> > verification procedure.
> > 
> > This patch adds a new func= for IMA appraisal specifically for the original
> > kexec_load syscall. Therefore, the kexec_load syscall can be effectively
> > disabled via IMA policy, leaving the kexec_file_load syscall able to do its
> > own signature verification, and not require it to be signed via IMA. To
> > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is
> > unchanged, and thus enables appraisal for both kexec syscalls.
> 
> This seems like a roundabout way to disallow the kexec_load syscall.
> Wouldn't it make more sense to simply disallow kexec_load any time
> CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that
> option impotent? Or has that idea already been rejected?

Agreed!  We can modify the "case LOADING_KEXEC_IMAGE" in
ima_load_data() to prevent the kexec_load based on
CONFIG_KEXEC_VERIFY_SIG.

The architecture specific policy would only include the IMA appraise
rule if CONFIG_KEXEC_VERIFY_SIG was not defined.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Seth Forshee Aug. 3, 2018, 4:16 p.m. UTC | #3
On Fri, Aug 03, 2018 at 10:54:59AM -0400, Mimi Zohar wrote:
> On Fri, 2018-08-03 at 08:11 -0500, Seth Forshee wrote:
> > On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote:
> > > IMA can verify the signature of kernel images loaded with kexec_file_load,
> > > but can not verify images loaded with the regular kexec_load syscall.
> > > Therefore, the appraisal will automatically fail during kexec_load when an
> > > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used
> > > to effectively disable the kexec_load syscall, while still allowing the
> > > kexec_file_load to operate so long as the target kernel image is signed.
> > > 
> > > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is
> > > enabled and there is an appraise rule set, then the target kernel would
> > > have to be verifiable by both IMA and the architecture specific kernel
> > > verification procedure.
> > > 
> > > This patch adds a new func= for IMA appraisal specifically for the original
> > > kexec_load syscall. Therefore, the kexec_load syscall can be effectively
> > > disabled via IMA policy, leaving the kexec_file_load syscall able to do its
> > > own signature verification, and not require it to be signed via IMA. To
> > > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is
> > > unchanged, and thus enables appraisal for both kexec syscalls.
> > 
> > This seems like a roundabout way to disallow the kexec_load syscall.
> > Wouldn't it make more sense to simply disallow kexec_load any time
> > CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that
> > option impotent? Or has that idea already been rejected?
> 
> Agreed!  We can modify the "case LOADING_KEXEC_IMAGE" in
> ima_load_data() to prevent the kexec_load based on
> CONFIG_KEXEC_VERIFY_SIG.
> 
> The architecture specific policy would only include the IMA appraise
> rule if CONFIG_KEXEC_VERIFY_SIG was not defined.

After looking at this some more I'm having second thoughts about my
suggestion. As a distro we produce a kernel that needs to be flexible
enough for a variety of scenarios, and if we completely close off the
ability to load an unsigned kernel for kexec that's almost certainly
going to end up breaking some use cases.

So I think it is necessary to make this a run-time decision rather than
a compile-time decision. The patch as provided does this based on
whether or not the kernel was booted under secure boot. That might be
reasonable, though I still find this mechanism kind of awkward. It seems
like ideally there would instead be some logic that would accept the
image if the KEXEC_VERIFY_SIG verification had passed, and otherwise
require IMA signature verification.

Thanks,
Seth
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Aug. 3, 2018, 7:47 p.m. UTC | #4
On Fri, 2018-08-03 at 11:16 -0500, Seth Forshee wrote:
> On Fri, Aug 03, 2018 at 10:54:59AM -0400, Mimi Zohar wrote:
> > On Fri, 2018-08-03 at 08:11 -0500, Seth Forshee wrote:
> > > On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote:
> > > > IMA can verify the signature of kernel images loaded with kexec_file_load,
> > > > but can not verify images loaded with the regular kexec_load syscall.
> > > > Therefore, the appraisal will automatically fail during kexec_load when an
> > > > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used
> > > > to effectively disable the kexec_load syscall, while still allowing the
> > > > kexec_file_load to operate so long as the target kernel image is signed.
> > > > 
> > > > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is
> > > > enabled and there is an appraise rule set, then the target kernel would
> > > > have to be verifiable by both IMA and the architecture specific kernel
> > > > verification procedure.
> > > > 
> > > > This patch adds a new func= for IMA appraisal specifically for the original
> > > > kexec_load syscall. Therefore, the kexec_load syscall can be effectively
> > > > disabled via IMA policy, leaving the kexec_file_load syscall able to do its
> > > > own signature verification, and not require it to be signed via IMA. To
> > > > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is
> > > > unchanged, and thus enables appraisal for both kexec syscalls.
> > > 
> > > This seems like a roundabout way to disallow the kexec_load syscall.
> > > Wouldn't it make more sense to simply disallow kexec_load any time
> > > CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that
> > > option impotent? Or has that idea already been rejected?
> > 
> > Agreed!  We can modify the "case LOADING_KEXEC_IMAGE" in
> > ima_load_data() to prevent the kexec_load based on
> > CONFIG_KEXEC_VERIFY_SIG.
> > 
> > The architecture specific policy would only include the IMA appraise
> > rule if CONFIG_KEXEC_VERIFY_SIG was not defined.
> 
> After looking at this some more I'm having second thoughts about my
> suggestion. As a distro we produce a kernel that needs to be flexible
> enough for a variety of scenarios, and if we completely close off the
> ability to load an unsigned kernel for kexec that's almost certainly
> going to end up breaking some use cases.
> 
> So I think it is necessary to make this a run-time decision rather than
> a compile-time decision. The patch as provided does this based on
> whether or not the kernel was booted under secure boot. That might be
> reasonable, though I still find this mechanism kind of awkward.

Right, the above change is almost right.  Instead of preventing the
kexec_load syscall based on CONFIG_KEXEC_VERIFY_SIG it should be based
on a runtime secure boot flag.  Only if there is an arch specific
secure boot function and the secure boot flag is enabled, would the
kexec_load be disabled.

Without an architecture specific secure boot function, the existing
IMA code would fail the kexec_load syscall.

>  It seems
> like ideally there would instead be some logic that would accept the
> image if the KEXEC_VERIFY_SIG verification had passed, and otherwise
> require IMA signature verification.

True, but for now to coordinate between the two signature verification
methods, only if CONFIG_KEXEC_VERIFY_SIG is not enabled would an IMA
architecture specific rule be defined.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox series

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 74c6702de74..031417779ec 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,6 +29,7 @@  Description:
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
+				[KEXEC_ORIG_KERNEL_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 6e5fa7c4280..c76e53c982b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -181,6 +181,7 @@  static inline unsigned long ima_hash_key(u8 *digest)
 	hook(MODULE_CHECK)		\
 	hook(FIRMWARE_CHECK)		\
 	hook(KEXEC_KERNEL_CHECK)	\
+	hook(KEXEC_ORIG_KERNEL_CHECK)	\
 	hook(KEXEC_INITRAMFS_CHECK)	\
 	hook(POLICY_CHECK)		\
 	hook(MAX_CHECK)
@@ -233,6 +234,7 @@  int ima_policy_show(struct seq_file *m, void *v);
 #define IMA_APPRAISE_FIRMWARE	0x10
 #define IMA_APPRAISE_POLICY	0x20
 #define IMA_APPRAISE_KEXEC	0x40
+#define IMA_APPRAISE_ORIG_KEXEC	0x80
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(enum ima_hooks func,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dce0a8a217b..a7b4220043d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -512,7 +512,8 @@  int ima_load_data(enum kernel_load_data_id id)
 
 	switch (id) {
 	case LOADING_KEXEC_IMAGE:
-		if (ima_appraise & IMA_APPRAISE_KEXEC) {
+		if (ima_appraise &
+		    (IMA_APPRAISE_ORIG_KEXEC | IMA_APPRAISE_KEXEC)) {
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 402e5bd1093..7a33e3f6eca 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -475,6 +475,8 @@  static int ima_appraise_flag(enum ima_hooks func)
 		return IMA_APPRAISE_POLICY;
 	else if (func == KEXEC_KERNEL_CHECK)
 		return IMA_APPRAISE_KEXEC;
+	else if (func == KEXEC_ORIG_KERNEL_CHECK)
+		return IMA_APPRAISE_ORIG_KEXEC;
 	return 0;
 }
 
@@ -879,6 +881,9 @@  static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") ==
 				 0)
 				entry->func = KEXEC_KERNEL_CHECK;
+			else if (strcmp(args[0].from,
+				 "KEXEC_ORIG_KERNEL_CHECK") == 0)
+				entry->func = KEXEC_ORIG_KERNEL_CHECK;
 			else if (strcmp(args[0].from, "KEXEC_INITRAMFS_CHECK")
 				 == 0)
 				entry->func = KEXEC_INITRAMFS_CHECK;