[v4a,1/2] selftests/kexec: make tests independent of IMA being enabled
diff mbox series

Message ID 1553283351-6310-1-git-send-email-zohar@linux.ibm.com
State New
Headers show
Series
  • [v4a,1/2] selftests/kexec: make tests independent of IMA being enabled
Related show

Commit Message

Mimi Zohar March 22, 2019, 7:35 p.m. UTC
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 <dyoung@redhat.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
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(-)

Comments

Dave Young March 25, 2019, 8:09 a.m. UTC | #1
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 <dyoung@redhat.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> 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
>
Mimi Zohar March 25, 2019, 8:37 p.m. UTC | #2
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 <dyoung@redhat.com>
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> > 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
Dave Young March 26, 2019, 7:49 a.m. UTC | #3
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 <dyoung@redhat.com>
> > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > ---
> > > 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
Mimi Zohar March 26, 2019, 1:56 p.m. UTC | #4
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 <dyoung@redhat.com>
> > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > ---
> > > > 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

Patch
diff mbox series

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"