2019-03-22 19:37:43

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled

Verify IMA is enabled before failing tests or emitting irrelevant
messages. Also, don't skip the test if signatures are not required.

Suggested-by: Dave Young <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
Dave, if this patch resolves the outstanding issues, I can fold these
changes into the original patches. (Reminder, these patches will need to
be updated to support the "lockdown" patch set.)

.../selftests/kexec/test_kexec_file_load.sh | 27 ++++++++++++++--------
tools/testing/selftests/kexec/test_kexec_load.sh | 24 ++++++++++++-------
2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/kexec/test_kexec_file_load.sh b/tools/testing/selftests/kexec/test_kexec_file_load.sh
index 1d2e5e799523..57b636792086 100755
--- a/tools/testing/selftests/kexec/test_kexec_file_load.sh
+++ b/tools/testing/selftests/kexec/test_kexec_file_load.sh
@@ -110,11 +110,20 @@ kexec_file_load_test()
log_fail "$succeed_msg (missing IMA sig)"
fi

- if [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] \
- && [ $ima_read_policy -eq 0 ] && [ $ima_signed -eq 0 ]; then
+ if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \
+ && [ $ima_sig_required -eq 0 ] && [ $ima_signed -eq 0 ] \
+ && [ $ima_read_policy -eq 0 ]; then
log_fail "$succeed_msg (possibly missing IMA sig)"
fi

+ if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 0 ]; then
+ log_info "No signature verification required"
+ elif [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \
+ && [ $ima_sig_required -eq 0 ] && [ $ima_signed -eq 0 ] \
+ && [ $ima_read_policy -eq 1 ]; then
+ log_info "No signature verification required"
+ fi
+
log_pass "$succeed_msg"
fi

@@ -136,8 +145,9 @@ kexec_file_load_test()
log_pass "$failed_msg (missing IMA sig)"
fi

- if [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] \
- && [ $ima_read_policy -eq 0 ] && [ $ima_signed -eq 0 ]; then
+ if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \
+ && [ $ima_sig_required -eq 0 ] && [ $ima_read_policy -eq 0 ] \
+ && [ $ima_signed -eq 0 ]; then
log_pass "$failed_msg (possibly missing IMA sig)"
fi

@@ -157,6 +167,9 @@ if [ $? -eq 0 ]; then
fi

# Determine which kernel config options are enabled
+kconfig_enabled "CONFIG_IMA_APPRAISE=y" "IMA enabled"
+ima_appraise=$?
+
kconfig_enabled "CONFIG_IMA_ARCH_POLICY=y" \
"architecture specific policy enabled"
arch_policy=$?
@@ -178,12 +191,6 @@ ima_sig_required=$?
get_secureboot_mode
secureboot=$?

-if [ $secureboot -eq 0 ] && [ $arch_policy -eq 0 ] && \
- [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] && \
- [ $ima_read_policy -eq 1 ]; then
- log_skip "No signature verification required"
-fi
-
# Are there pe and ima signatures
check_for_pesig
pe_signed=$?
diff --git a/tools/testing/selftests/kexec/test_kexec_load.sh b/tools/testing/selftests/kexec/test_kexec_load.sh
index 2a66c8897f55..49c6aa929137 100755
--- a/tools/testing/selftests/kexec/test_kexec_load.sh
+++ b/tools/testing/selftests/kexec/test_kexec_load.sh
@@ -1,8 +1,8 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
-# Loading a kernel image via the kexec_load syscall should fail
-# when the kernel is CONFIG_KEXEC_VERIFY_SIG enabled and the system
-# is booted in secureboot mode.
+#
+# Prevent loading a kernel image via the kexec_load syscall when
+# signatures are required. (Dependent on CONFIG_IMA_ARCH_POLICY.)

TEST="$0"
. ./kexec_common_lib.sh
@@ -18,20 +18,28 @@ if [ $? -eq 0 ]; then
log_skip "kexec_load is not enabled"
fi

+kconfig_enabled "CONFIG_IMA_APPRAISE=y" "IMA enabled"
+ima_appraise=$?
+
+kconfig_enabled "CONFIG_IMA_ARCH_POLICY=y" \
+ "IMA architecture specific policy enabled"
+arch_policy=$?
+
get_secureboot_mode
secureboot=$?

-# kexec_load should fail in secure boot mode
+# kexec_load should fail in secure boot mode and CONFIG_IMA_ARCH_POLICY enabled
kexec --load $KERNEL_IMAGE > /dev/null 2>&1
if [ $? -eq 0 ]; then
kexec --unload
- if [ $secureboot -eq 1 ]; then
+ if [ $secureboot -eq 1 ] && [ $arch_policy -eq 1 ]; then
log_fail "kexec_load succeeded"
- else
- log_pass "kexec_load succeeded"
+ elif [ $ima_appraise -eq 0 -o $arch_policy -eq 0 ]; then
+ log_info "Either IMA or the IMA arch policy is not enabled"
fi
+ log_pass "kexec_load succeeded"
else
- if [ $secureboot -eq 1 ]; then
+ if [ $secureboot -eq 1 ] && [ $arch_policy -eq 1 ] ; then
log_pass "kexec_load failed"
else
log_fail "kexec_load failed"
--
2.7.5



2019-03-22 19:37:45

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v4a 2/2] selftests/kexec: testing CONFIG_KEXEC_BZIMAGE_VERIFY_SIG is not enough

Add support for CONFIG_KEXEC_VERIFY_SIG being enabled, but not
CONFIG_KEXEC_BZIMAGE_VERIFY_SIG.

Signed-off-by: Mimi Zohar <[email protected]>
---
tools/testing/selftests/kexec/test_kexec_file_load.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kexec/test_kexec_file_load.sh b/tools/testing/selftests/kexec/test_kexec_file_load.sh
index 57b636792086..fa7c24e8eefb 100755
--- a/tools/testing/selftests/kexec/test_kexec_file_load.sh
+++ b/tools/testing/selftests/kexec/test_kexec_file_load.sh
@@ -102,7 +102,8 @@ kexec_file_load_test()
log_fail "$succeed_msg (missing sig)"
fi

- if [ $pe_sig_required -eq 1 ] && [ $pe_signed -eq 0 ]; then
+ if [ $kexec_sig_required -eq 1 -o $pe_sig_required -eq 1 ] \
+ && [ $pe_signed -eq 0 ]; then
log_fail "$succeed_msg (missing PE sig)"
fi

@@ -137,7 +138,8 @@ kexec_file_load_test()
fi
fi

- if [ $pe_sig_required -eq 1 ] && [ $pe_signed -eq 0 ]; then
+ if [ $kexec_sig_required -eq 1 -o $pe_sig_required -eq 1 ] \
+ && [ $pe_signed -eq 0 ]; then
log_pass "$failed_msg (missing PE sig)"
fi

@@ -181,6 +183,10 @@ platform_keyring=$?
kconfig_enabled "CONFIG_IMA_READ_POLICY=y" "reading IMA policy permitted"
ima_read_policy=$?

+kconfig_enabled "CONFIG_KEXEC_SIG_FORCE=y" \
+ "kexec signed kernel image required"
+kexec_sig_required=$?
+
kconfig_enabled "CONFIG_KEXEC_BZIMAGE_VERIFY_SIG=y" \
"PE signed kernel image required"
pe_sig_required=$?
--
2.7.5


2019-03-25 08:11:17

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled

Hi Mimi
On 03/22/19 at 03:35pm, Mimi Zohar wrote:
> Verify IMA is enabled before failing tests or emitting irrelevant
> messages. Also, don't skip the test if signatures are not required.
>
> Suggested-by: Dave Young <[email protected]>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> Dave, if this patch resolves the outstanding issues, I can fold these
> changes into the original patches. (Reminder, these patches will need to
> be updated to support the "lockdown" patch set.)

They looks good to me, thanks for the update

Feel free to add my reviewed-by, I did some tests although not cover all
ima cases.

Thanks
Dave

>
> .../selftests/kexec/test_kexec_file_load.sh | 27 ++++++++++++++--------
> tools/testing/selftests/kexec/test_kexec_load.sh | 24 ++++++++++++-------
> 2 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/kexec/test_kexec_file_load.sh b/tools/testing/selftests/kexec/test_kexec_file_load.sh
> index 1d2e5e799523..57b636792086 100755
> --- a/tools/testing/selftests/kexec/test_kexec_file_load.sh
> +++ b/tools/testing/selftests/kexec/test_kexec_file_load.sh
> @@ -110,11 +110,20 @@ kexec_file_load_test()
> log_fail "$succeed_msg (missing IMA sig)"
> fi
>
> - if [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] \
> - && [ $ima_read_policy -eq 0 ] && [ $ima_signed -eq 0 ]; then
> + if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \
> + && [ $ima_sig_required -eq 0 ] && [ $ima_signed -eq 0 ] \
> + && [ $ima_read_policy -eq 0 ]; then
> log_fail "$succeed_msg (possibly missing IMA sig)"
> fi
>
> + if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 0 ]; then
> + log_info "No signature verification required"
> + elif [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \
> + && [ $ima_sig_required -eq 0 ] && [ $ima_signed -eq 0 ] \
> + && [ $ima_read_policy -eq 1 ]; then
> + log_info "No signature verification required"
> + fi
> +
> log_pass "$succeed_msg"
> fi
>
> @@ -136,8 +145,9 @@ kexec_file_load_test()
> log_pass "$failed_msg (missing IMA sig)"
> fi
>
> - if [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] \
> - && [ $ima_read_policy -eq 0 ] && [ $ima_signed -eq 0 ]; then
> + if [ $pe_sig_required -eq 0 ] && [ $ima_appraise -eq 1 ] \
> + && [ $ima_sig_required -eq 0 ] && [ $ima_read_policy -eq 0 ] \
> + && [ $ima_signed -eq 0 ]; then
> log_pass "$failed_msg (possibly missing IMA sig)"
> fi
>
> @@ -157,6 +167,9 @@ if [ $? -eq 0 ]; then
> fi
>
> # Determine which kernel config options are enabled
> +kconfig_enabled "CONFIG_IMA_APPRAISE=y" "IMA enabled"
> +ima_appraise=$?
> +
> kconfig_enabled "CONFIG_IMA_ARCH_POLICY=y" \
> "architecture specific policy enabled"
> arch_policy=$?
> @@ -178,12 +191,6 @@ ima_sig_required=$?
> get_secureboot_mode
> secureboot=$?
>
> -if [ $secureboot -eq 0 ] && [ $arch_policy -eq 0 ] && \
> - [ $pe_sig_required -eq 0 ] && [ $ima_sig_required -eq 0 ] && \
> - [ $ima_read_policy -eq 1 ]; then
> - log_skip "No signature verification required"
> -fi
> -
> # Are there pe and ima signatures
> check_for_pesig
> pe_signed=$?
> diff --git a/tools/testing/selftests/kexec/test_kexec_load.sh b/tools/testing/selftests/kexec/test_kexec_load.sh
> index 2a66c8897f55..49c6aa929137 100755
> --- a/tools/testing/selftests/kexec/test_kexec_load.sh
> +++ b/tools/testing/selftests/kexec/test_kexec_load.sh
> @@ -1,8 +1,8 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0
> -# Loading a kernel image via the kexec_load syscall should fail
> -# when the kernel is CONFIG_KEXEC_VERIFY_SIG enabled and the system
> -# is booted in secureboot mode.
> +#
> +# Prevent loading a kernel image via the kexec_load syscall when
> +# signatures are required. (Dependent on CONFIG_IMA_ARCH_POLICY.)
>
> TEST="$0"
> . ./kexec_common_lib.sh
> @@ -18,20 +18,28 @@ if [ $? -eq 0 ]; then
> log_skip "kexec_load is not enabled"
> fi
>
> +kconfig_enabled "CONFIG_IMA_APPRAISE=y" "IMA enabled"
> +ima_appraise=$?
> +
> +kconfig_enabled "CONFIG_IMA_ARCH_POLICY=y" \
> + "IMA architecture specific policy enabled"
> +arch_policy=$?
> +
> get_secureboot_mode
> secureboot=$?
>
> -# kexec_load should fail in secure boot mode
> +# kexec_load should fail in secure boot mode and CONFIG_IMA_ARCH_POLICY enabled
> kexec --load $KERNEL_IMAGE > /dev/null 2>&1
> if [ $? -eq 0 ]; then
> kexec --unload
> - if [ $secureboot -eq 1 ]; then
> + if [ $secureboot -eq 1 ] && [ $arch_policy -eq 1 ]; then
> log_fail "kexec_load succeeded"
> - else
> - log_pass "kexec_load succeeded"
> + elif [ $ima_appraise -eq 0 -o $arch_policy -eq 0 ]; then
> + log_info "Either IMA or the IMA arch policy is not enabled"
> fi
> + log_pass "kexec_load succeeded"
> else
> - if [ $secureboot -eq 1 ]; then
> + if [ $secureboot -eq 1 ] && [ $arch_policy -eq 1 ] ; then
> log_pass "kexec_load failed"
> else
> log_fail "kexec_load failed"
> --
> 2.7.5
>

2019-03-25 20:39:32

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled

On Mon, 2019-03-25 at 16:09 +0800, Dave Young wrote:
> Hi Mimi
> On 03/22/19 at 03:35pm, Mimi Zohar wrote:
> > Verify IMA is enabled before failing tests or emitting irrelevant
> > messages. Also, don't skip the test if signatures are not required.
> >
> > Suggested-by: Dave Young <[email protected]>
> > Signed-off-by: Mimi Zohar <[email protected]>
> > ---
> > Dave, if this patch resolves the outstanding issues, I can fold these
> > changes into the original patches. (Reminder, these patches will need to
> > be updated to support the "lockdown" patch set.)
>
> They looks good to me, thanks for the update

I've folded the kexec_file_load changes into the kexec_file_load test.
 The remaining kexec_load change is left as a separate patch, since it
is dependent on the ikconfig change.

> Feel free to add my reviewed-by, I did some tests although not cover all
> ima cases.

Thanks!  Is this meant as a general "reviewed-by" for all of the
patches or just this specific one?

Mimi


2019-03-26 07:51:07

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled

On 03/25/19 at 04:37pm, Mimi Zohar wrote:
> On Mon, 2019-03-25 at 16:09 +0800, Dave Young wrote:
> > Hi Mimi
> > On 03/22/19 at 03:35pm, Mimi Zohar wrote:
> > > Verify IMA is enabled before failing tests or emitting irrelevant
> > > messages. Also, don't skip the test if signatures are not required.
> > >
> > > Suggested-by: Dave Young <[email protected]>
> > > Signed-off-by: Mimi Zohar <[email protected]>
> > > ---
> > > Dave, if this patch resolves the outstanding issues, I can fold these
> > > changes into the original patches. (Reminder, these patches will need to
> > > be updated to support the "lockdown" patch set.)
> >
> > They looks good to me, thanks for the update
>
> I've folded the kexec_file_load changes into the kexec_file_load test.
> ?The remaining kexec_load change is left as a separate patch, since it
> is dependent on the ikconfig change.
>
> > Feel free to add my reviewed-by, I did some tests although not cover all
> > ima cases.
>
> Thanks! ?Is this meant as a general "reviewed-by" for all of the
> patches or just this specific one?

Thank you for taking this as a separate kexec tests, I think it can be used for these delta fixes

I read all the patches and reviewed the kexec stuff, but I do not
understand all the IMA logic yet although I did some simple ima
tests.

Thanks
Dave

2019-03-26 13:59:26

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v4a 1/2] selftests/kexec: make tests independent of IMA being enabled

On Tue, 2019-03-26 at 15:49 +0800, Dave Young wrote:
> On 03/25/19 at 04:37pm, Mimi Zohar wrote:
> > On Mon, 2019-03-25 at 16:09 +0800, Dave Young wrote:
> > > Hi Mimi
> > > On 03/22/19 at 03:35pm, Mimi Zohar wrote:
> > > > Verify IMA is enabled before failing tests or emitting irrelevant
> > > > messages. Also, don't skip the test if signatures are not required.
> > > >
> > > > Suggested-by: Dave Young <[email protected]>
> > > > Signed-off-by: Mimi Zohar <[email protected]>
> > > > ---
> > > > Dave, if this patch resolves the outstanding issues, I can fold these
> > > > changes into the original patches. (Reminder, these patches will need to
> > > > be updated to support the "lockdown" patch set.)
> > >
> > > They looks good to me, thanks for the update
> >
> > I've folded the kexec_file_load changes into the kexec_file_load test.
> >  The remaining kexec_load change is left as a separate patch, since it
> > is dependent on the ikconfig change.
> >
> > > Feel free to add my reviewed-by, I did some tests although not cover all
> > > ima cases.
> >
> > Thanks!  Is this meant as a general "reviewed-by" for all of the
> > patches or just this specific one?
>
> Thank you for taking this as a separate kexec tests, I think it can
> be used for these delta fixes

Ok, I just re-posted the patches, folding part of this patch into the
kexec_file_load test.  I've added your Reviewed-by on the remaining
patch.

>
> I read all the patches and reviewed the kexec stuff, but I do not
> understand all the IMA logic yet although I did some simple ima
> tests.

I understand.  There are many different aspects to the integrity
subsystem.  I'm happy to answer any questions you have.

Mimi