[PATCH 0/3] selftest/ima: fail kexec_load syscall

[PATCH 0/3] selftest/ima: fail kexec_load syscall

[Mimi Zohar]

The "ima: add support for arch specific policies" patch set introduced
architecture specific policies, including an x86 policy which prevents
loading a kernel image via the kexec_load syscall.

This patch set preq's that patch set, adding a missing kexec_load
syscall failure message, extending the existing support for detecting
secureboot mode, and defining a kexec_load syscall selftest to
simplify testing.

To run the kexec_load test requires root privileges.  Execute:
	"sudo make TARGETS=ima kselftest".

With secure boot enabled, the kexec_load fails, but the test
succeeds.

selftests: ima: test_kexec_load.sh
========================================
./test_kexec_load.sh: kexec_load failed [PASS]
ok 1..1 selftests: ima: test_kexec_load.sh [PASS]


Mimi

Mimi Zohar (3):
  ima: add error mesage to kexec_load
  selftests/ima: kexec_load syscall test
  x86/ima: retry detecting secure boot mode

 arch/x86/kernel/Makefile                       |  2 +
 arch/x86/kernel/ima_arch.c                     | 46 +++++++++++++++++++++-
 include/linux/ima.h                            |  2 +-
 security/integrity/ima/ima_main.c              |  4 +-
 tools/testing/selftests/Makefile               |  1 +
 tools/testing/selftests/ima/Makefile           | 11 ++++++
 tools/testing/selftests/ima/config             |  4 ++
 tools/testing/selftests/ima/test_kexec_load.sh | 54 ++++++++++++++++++++++++++
 8 files changed, 120 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/ima/Makefile
 create mode 100644 tools/testing/selftests/ima/config
 create mode 100755 tools/testing/selftests/ima/test_kexec_load.sh

-- 
2.7.5

[PATCH 1/3] ima: add error mesage to kexec_load

[Mimi Zohar]

Reject the kexec_load syscall with some indication of the problem.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 41e4771980d5..df0b2ee49fa2 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -513,8 +513,10 @@ int ima_load_data(enum kernel_load_data_id id)
 	switch (id) {
 	case LOADING_KEXEC_IMAGE:
 		if (IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG)
-		    && arch_ima_get_secureboot())
+		    && arch_ima_get_secureboot()) {
+			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n");
 			return -EACCES;
+		}
 
 		if (ima_enforce && (ima_appraise & IMA_APPRAISE_KEXEC)) {
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n");
-- 
2.7.5

[PATCH 2/3] selftests/ima: kexec_load syscall test

[Mimi Zohar]

The kernel CONFIG_KEXEC_VERIFY_SIG option is limited to verifying a
kernel image's signature, when loaded via the kexec_file_load syscall.
There is no method for verifying a kernel image's signature loaded
via the kexec_load syscall.

This test verifies loading the kernel image via the kexec_load syscall
fails when the kernel CONFIG_KEXEC_VERIFY_SIG option is enabled on
systems with secureboot enabled[1].

[1] Detecting secureboot enabled is architecture specific.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 tools/testing/selftests/Makefile               |  1 +
 tools/testing/selftests/ima/Makefile           | 11 ++++++
 tools/testing/selftests/ima/config             |  4 ++
 tools/testing/selftests/ima/test_kexec_load.sh | 54 ++++++++++++++++++++++++++
 4 files changed, 70 insertions(+)
 create mode 100644 tools/testing/selftests/ima/Makefile
 create mode 100644 tools/testing/selftests/ima/config
 create mode 100755 tools/testing/selftests/ima/test_kexec_load.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f1fe492c8e17..552c447af03d 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -13,6 +13,7 @@ TARGETS += firmware
 TARGETS += ftrace
 TARGETS += futex
 TARGETS += gpio
+TARGETS += ima
 TARGETS += intel_pstate
 TARGETS += ipc
 TARGETS += kcmp
diff --git a/tools/testing/selftests/ima/Makefile b/tools/testing/selftests/ima/Makefile
new file mode 100644
index 000000000000..0b3adf5444b6
--- /dev/null
+++ b/tools/testing/selftests/ima/Makefile
@@ -0,0 +1,11 @@
+# Makefile for kexec_load
+
+uname_M := $(shell uname -m 2>/dev/null || echo not)
+ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
+
+ifeq ($(ARCH),x86)
+TEST_PROGS := test_kexec_load.sh
+
+include ../lib.mk
+
+endif
diff --git a/tools/testing/selftests/ima/config b/tools/testing/selftests/ima/config
new file mode 100644
index 000000000000..6bc86d4d9bb4
--- /dev/null
+++ b/tools/testing/selftests/ima/config
@@ -0,0 +1,4 @@
+CONFIG_IMA_APPRAISE
+CONFIG_IMA_ARCH_POLICY
+CONFIG_SECURITYFS
+CONFIG_KEXEC_VERIFY_SIG
diff --git a/tools/testing/selftests/ima/test_kexec_load.sh b/tools/testing/selftests/ima/test_kexec_load.sh
new file mode 100755
index 000000000000..3aafc6f6a400
--- /dev/null
+++ b/tools/testing/selftests/ima/test_kexec_load.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Loading a kernel image via the kexec_load syscall should fail
+# when the kerne is CONFIG_KEXEC_VERIFY_SIG enabled and the system
+# is booted in secureboot mode.
+
+TEST="$0"
+EFIVARFS="/sys/firmware/efi/efivars"
+rc=0
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+# kexec requires root privileges
+if [ $UID != 0 ]; then
+	echo "$TEST: must be run as root" >&2
+	exit $ksft_skip
+fi
+
+# Make sure that efivars is mounted in the normal location
+if ! grep -q "^\S\+ $EFIVARFS efivarfs" /proc/mounts; then
+	echo "$TEST: efivars is not mounted on $EFIVARFS" >&2
+	exit $ksft_skip
+fi
+
+# Get secureboot mode
+file="$EFIVARFS/SecureBoot-*"
+if [ ! -e $file ]; then
+	echo "$TEST: unknown secureboot mode" >&2
+	exit $ksft_skip
+fi
+secureboot=`hexdump $file | awk '{print substr($4,length($4),1)}'`
+
+# kexec_load should fail in secure boot mode
+KERNEL_IMAGE="/boot/vmlinuz-`uname -r`"
+kexec -l $KERNEL_IMAGE &>> /dev/null
+if [ $? == 0 ]; then
+	kexec -u
+	if [ "$secureboot" == "1" ]; then
+		echo "$TEST: kexec_load succeeded [FAIL]"
+		rc=1
+	else
+		echo "$TEST: kexec_load succeeded [PASS]"
+	fi
+else
+	if [ "$secureboot" == "1" ]; then
+		echo "$TEST: kexec_load failed [PASS]"
+	else
+		echo "$TEST: kexec_load failed [FAIL]"
+		rc=1
+	fi
+fi
+
+exit $rc
-- 
2.7.5

[PATCH 3/3] x86/ima: retry detecting secure boot mode

[Mimi Zohar]

The secure boot mode may not be detected on boot for some reason (eg.
buggy firmware).  This patch attempts one more time to detect the
secure boot mode.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 arch/x86/kernel/Makefile   |  2 ++
 arch/x86/kernel/ima_arch.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/ima.h        |  2 +-
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index f0910a1e1db7..eb51b0e1189c 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -151,4 +151,6 @@ ifeq ($(CONFIG_X86_64),y)
 	obj-y				+= vsmp_64.o
 endif
 
+ifdef CONFIG_EFI
 obj-$(CONFIG_IMA)			+= ima_arch.o
+endif
diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
index 6c248616ee57..e47cd9390ab4 100644
--- a/arch/x86/kernel/ima_arch.c
+++ b/arch/x86/kernel/ima_arch.c
@@ -7,10 +7,52 @@
 
 extern struct boot_params boot_params;
 
+static enum efi_secureboot_mode get_sb_mode(void)
+{
+	efi_char16_t efi_SecureBoot_name[] = L"SecureBoot";
+	efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+	efi_status_t status;
+	unsigned long size;
+	u8 secboot;
+
+	size = sizeof(secboot);
+
+	/* Get variable contents into buffer */
+	status = efi.get_variable(efi_SecureBoot_name, &efi_variable_guid,
+				  NULL, &size, &secboot);
+	if (status == EFI_NOT_FOUND) {
+		pr_info("ima: secureboot mode disabled\n");
+		return efi_secureboot_mode_disabled;
+	}
+
+	if (status != EFI_SUCCESS) {
+		pr_info("ima: secureboot mode unknown\n");
+		return efi_secureboot_mode_unknown;
+	}
+
+	if (secboot == 0) {
+		pr_info("ima: secureboot mode disabled\n");
+		return efi_secureboot_mode_disabled;
+	}
+
+	pr_info("ima: secureboot mode enabled\n");
+	return efi_secureboot_mode_enabled;
+}
+
 bool arch_ima_get_secureboot(void)
 {
-	if (efi_enabled(EFI_BOOT) &&
-		(boot_params.secure_boot == efi_secureboot_mode_enabled))
+	static enum efi_secureboot_mode sb_mode;
+	static bool initialized;
+
+	if (!initialized && efi_enabled(EFI_BOOT)) {
+		sb_mode = boot_params.secure_boot;
+
+		if (sb_mode == efi_secureboot_mode_unset)
+			sb_mode = get_sb_mode();
+		initialized = true;
+	}
+
+	if (sb_mode == efi_secureboot_mode_enabled)
 		return true;
 	else
 		return false;
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 5ab9134d4fd7..b5e16b8c50b7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -30,7 +30,7 @@ extern void ima_post_path_mknod(struct dentry *dentry);
 extern void ima_add_kexec_buffer(struct kimage *image);
 #endif
 
-#ifdef CONFIG_X86
+#if defined(CONFIG_X86) && defined(CONFIG_EFI)
 extern bool arch_ima_get_secureboot(void);
 extern const char * const *arch_get_ima_policy(void);
 #else
-- 
2.7.5

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Matthew Garrett]

On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> The secure boot mode may not be detected on boot for some reason (eg.
> buggy firmware).  This patch attempts one more time to detect the
> secure boot mode.

Do we have cases where this has actually been seen? I'm not sure what
the circumstances are that would result in this behaviour.

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Matthew Garrett]

On Thu, Mar 7, 2019 at 2:38 PM Justin Forbes <jforbes@redhat.com> wrote:
> On Thu, Mar 7, 2019 at 4:29 PM Matthew Garrett <mjg59@google.com> wrote:
>>
>> On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>> >
>> > The secure boot mode may not be detected on boot for some reason (eg.
>> > buggy firmware).  This patch attempts one more time to detect the
>> > secure boot mode.
>>
>> Do we have cases where this has actually been seen? I'm not sure what
>> the circumstances are that would result in this behaviour.
>
>
> We have never seen it in practice, though we only ever do anything with it with x86, so it is possible that some other platforms maybe?

I'm not sure that it buys us anything to check this in both the boot
stub and the running kernel. If a platform *is* giving us different
results, anything else relying on the information from the boot stub
is also going to be broken, so we should do this centrally rather than
in the IMA code.

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Mimi Zohar]

On Thu, 2019-03-07 at 14:44 -0800, Matthew Garrett wrote:
> On Thu, Mar 7, 2019 at 2:38 PM Justin Forbes <jforbes@redhat.com> wrote:
> > On Thu, Mar 7, 2019 at 4:29 PM Matthew Garrett <mjg59@google.com> wrote:
> >>
> >> On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >> >
> >> > The secure boot mode may not be detected on boot for some reason (eg.
> >> > buggy firmware).  This patch attempts one more time to detect the
> >> > secure boot mode.
> >>
> >> Do we have cases where this has actually been seen? I'm not sure what
> >> the circumstances are that would result in this behaviour.
> >
> >
> > We have never seen it in practice, though we only ever do anything with it with x86, so it is possible that some other platforms maybe?
> 
> I'm not sure that it buys us anything to check this in both the boot
> stub and the running kernel. If a platform *is* giving us different
> results, anything else relying on the information from the boot stub
> is also going to be broken, so we should do this centrally rather than
> in the IMA code.

I added this last attempt because I'm seeing this on my laptop, with
some older, buggy firmware.

Mimi

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Matthew Garrett]

On Thu, Mar 7, 2019 at 2:48 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> I added this last attempt because I'm seeing this on my laptop, with
> some older, buggy firmware.

Is the issue that it gives incorrect results on the first read, or is
the issue that it gives incorrect results before ExitBootServices() is
called? If the former then we should read twice in the boot stub, if
the latter then we should figure out a way to do this immediately
after ExitBootServices() instead.

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Mimi Zohar]

On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote:
> On Thu, Mar 7, 2019 at 2:48 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > I added this last attempt because I'm seeing this on my laptop, with
> > some older, buggy firmware.
> 
> Is the issue that it gives incorrect results on the first read, or is
> the issue that it gives incorrect results before ExitBootServices() is
> called? If the former then we should read twice in the boot stub, if
> the latter then we should figure out a way to do this immediately
> after ExitBootServices() instead.

Detecting the secure boot mode isn't the problem.  On boot, I am
seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits
"Secure boot could not be determined".

In efi_main() the secure_boot mode is initially unset, so
efi_get_secureboot() is called.  efi_get_secureboot() returns the
secure_boot mode correctly as enabled.  The problem seems to be in
saving the secure_boot mode for later use.

Mimi

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Matthew Garrett]

On Fri, Mar 8, 2019 at 5:40 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote:
> > Is the issue that it gives incorrect results on the first read, or is
> > the issue that it gives incorrect results before ExitBootServices() is
> > called? If the former then we should read twice in the boot stub, if
> > the latter then we should figure out a way to do this immediately
> > after ExitBootServices() instead.
>
> Detecting the secure boot mode isn't the problem.  On boot, I am
> seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits
> "Secure boot could not be determined".
>
> In efi_main() the secure_boot mode is initially unset, so
> efi_get_secureboot() is called.  efi_get_secureboot() returns the
> secure_boot mode correctly as enabled.  The problem seems to be in
> saving the secure_boot mode for later use.

Hm. And this only happens on certain firmware versions? If something's
stepping on boot_params then we have bigger problems.

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Mimi Zohar]

On Fri, 2019-03-08 at 09:51 -0800, Matthew Garrett wrote:
> On Fri, Mar 8, 2019 at 5:40 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote:
> > > Is the issue that it gives incorrect results on the first read, or is
> > > the issue that it gives incorrect results before ExitBootServices() is
> > > called? If the former then we should read twice in the boot stub, if
> > > the latter then we should figure out a way to do this immediately
> > > after ExitBootServices() instead.
> >
> > Detecting the secure boot mode isn't the problem.  On boot, I am
> > seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits
> > "Secure boot could not be determined".
> >
> > In efi_main() the secure_boot mode is initially unset, so
> > efi_get_secureboot() is called.  efi_get_secureboot() returns the
> > secure_boot mode correctly as enabled.  The problem seems to be in
> > saving the secure_boot mode for later use.
> 
> Hm. And this only happens on certain firmware versions? If something's
> stepping on boot_params then we have bigger problems.

FYI, efi_printk() works before exit_boot(), but not afterwards.  The
system hangs.

Mimi

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Matthew Garrett]

On Fri, Mar 8, 2019 at 10:43 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> FYI, efi_printk() works before exit_boot(), but not afterwards.  The
> system hangs.

efi_printk() uses boot services to print, so that's not unexpected :)
It would probably be sensible to return an error rather than crash,
though…

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Mimi Zohar]

On Fri, 2019-03-08 at 09:51 -0800, Matthew Garrett wrote:
> On Fri, Mar 8, 2019 at 5:40 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2019-03-07 at 14:50 -0800, Matthew Garrett wrote:
> > > Is the issue that it gives incorrect results on the first read, or is
> > > the issue that it gives incorrect results before ExitBootServices() is
> > > called? If the former then we should read twice in the boot stub, if
> > > the latter then we should figure out a way to do this immediately
> > > after ExitBootServices() instead.
> >
> > Detecting the secure boot mode isn't the problem.  On boot, I am
> > seeing "EFI stub: UEFI Secure Boot is enabled", but setup_arch() emits
> > "Secure boot could not be determined".
> >
> > In efi_main() the secure_boot mode is initially unset, so
> > efi_get_secureboot() is called.  efi_get_secureboot() returns the
> > secure_boot mode correctly as enabled.  The problem seems to be in
> > saving the secure_boot mode for later use.
> 
> Hm. And this only happens on certain firmware versions? If something's
> stepping on boot_params then we have bigger problems.

I was seeing this problem before and after updating the system
firmware on my laptop last summer.  If updating the firmware had
resolved the problem, I wouldn't have included this patch.

Mimi

Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode

[Matthew Garrett]

On Mon, Mar 11, 2019 at 9:55 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Fri, 2019-03-08 at 09:51 -0800, Matthew Garrett wrote:
> > Hm. And this only happens on certain firmware versions? If something's
> > stepping on boot_params then we have bigger problems.
>
> I was seeing this problem before and after updating the system
> firmware on my laptop last summer.  If updating the firmware had
> resolved the problem, I wouldn't have included this patch.

Ah, sorry, when you said that you saw this with older firmware I
thought that meant that newer firmware fixed it. What machine are you
testing on?