[2/3] scripts/ima: define a set of common functions
diff mbox series

Message ID 1548960936-7800-3-git-send-email-zohar@linux.ibm.com
State New
Headers show
Series
  • selftest/ima: add kexec_file_load test
Related show

Commit Message

Mimi Zohar Jan. 31, 2019, 6:55 p.m. UTC
Define and move get_secureboot_mode() to a common file for use by other
tests.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 tools/testing/selftests/ima/common_lib.sh      | 20 ++++++++++++++++++++
 tools/testing/selftests/ima/test_kexec_load.sh | 17 +++--------------
 2 files changed, 23 insertions(+), 14 deletions(-)
 create mode 100755 tools/testing/selftests/ima/common_lib.sh

Comments

Petr Vorel Feb. 3, 2019, 9:19 p.m. UTC | #1
Hi Mimi,

> Define and move get_secureboot_mode() to a common file for use by other
> tests.

> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> ---
>  tools/testing/selftests/ima/common_lib.sh      | 20 ++++++++++++++++++++
>  tools/testing/selftests/ima/test_kexec_load.sh | 17 +++--------------
>  2 files changed, 23 insertions(+), 14 deletions(-)
>  create mode 100755 tools/testing/selftests/ima/common_lib.sh

> diff --git a/tools/testing/selftests/ima/common_lib.sh b/tools/testing/selftests/ima/common_lib.sh
> new file mode 100755
> index 000000000000..ae097a634da5
> --- /dev/null
> +++ b/tools/testing/selftests/ima/common_lib.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0+
# SPDX-License-Identifier: GPL-2.0-or-later

> +
> +get_secureboot_mode()
> +{
> +	EFIVARFS="/sys/firmware/efi/efivars"
	local efivarfs="/sys/firmware/efi/efivars"
	local file
It's a good practise to use local keyword and lower case the name of the
variable for variables used only locally (if you treat $EFIVARFS as constant,
I'd move it outside of get_secureboot_mode()).
I personally try to avoid using global variables (except constant like).

> +	# 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
There could be helper function printing error and exit in selftest library.

> +	# Get secureboot mode
> +	file="$EFIVARFS/SecureBoot-*"
	file="$efivarfs/SecureBoot-*"

...
>  KERNEL_IMAGE="/boot/vmlinuz-`uname -r`"
Another candidate for helper for potential selftest library.

Kind regards,
Petr
Dave Young Feb. 28, 2019, 1:41 p.m. UTC | #2
Hi Mimi,
 
Sorry for jumping in late, just noticed this kexec selftests, I think we
also need a kexec load test not only for ima, but for general kexec

On 01/31/19 at 01:55pm, Mimi Zohar wrote:
> Define and move get_secureboot_mode() to a common file for use by other
> tests.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  tools/testing/selftests/ima/common_lib.sh      | 20 ++++++++++++++++++++
>  tools/testing/selftests/ima/test_kexec_load.sh | 17 +++--------------
>  2 files changed, 23 insertions(+), 14 deletions(-)
>  create mode 100755 tools/testing/selftests/ima/common_lib.sh
> 
> diff --git a/tools/testing/selftests/ima/common_lib.sh b/tools/testing/selftests/ima/common_lib.sh
> new file mode 100755
> index 000000000000..ae097a634da5
> --- /dev/null
> +++ b/tools/testing/selftests/ima/common_lib.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +get_secureboot_mode()
> +{
> +	EFIVARFS="/sys/firmware/efi/efivars"
> +	# 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
> +	return `hexdump $file | awk '{print substr($4,length($4),1)}'`
> +}

Do you want to get the Secureboot status here?
I got some advice from Peter Jones previously, thus we have below
in our kdump scripts:
https://src.fedoraproject.org/cgit/rpms/kexec-tools.git/tree/kdump-lib.sh
 
See the function is_secure_boot_enforced(), probably you can refer to
that function and check setup mode as well.

> diff --git a/tools/testing/selftests/ima/test_kexec_load.sh b/tools/testing/selftests/ima/test_kexec_load.sh
> index 74423c4229e2..5e3566738888 100755
> --- a/tools/testing/selftests/ima/test_kexec_load.sh
> +++ b/tools/testing/selftests/ima/test_kexec_load.sh
> @@ -5,7 +5,7 @@
>  # is booted in secureboot mode.
>  
>  TEST="$0"
> -EFIVARFS="/sys/firmware/efi/efivars"
> +. ./common_lib.sh
>  rc=0
>  
>  # Kselftest framework requirement - SKIP code is 4.
> @@ -17,19 +17,8 @@ if [ $(id -ru) != 0 ]; then
>  	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)}'`
> +get_secureboot_mode
> +secureboot=$?
>  
>  # kexec_load should fail in secure boot mode
>  KERNEL_IMAGE="/boot/vmlinuz-`uname -r`"
> -- 
> 2.7.5
> 
Thanks
Dave
Mimi Zohar Feb. 28, 2019, 3:05 p.m. UTC | #3
Hi Dave,

On Thu, 2019-02-28 at 21:41 +0800, Dave Young wrote:
> Hi Mimi,
>  
> Sorry for jumping in late, just noticed this kexec selftests, I think we
> also need a kexec load test not only for ima, but for general kexec

The IMA kselftest tests are for the coordination between the different
methods of verifying file signatures.  In particular, for the kexec
kernel image and kernel module signatures.

The initial IMA kselftest just verifies that in an environment
requiring signed kexec kernel images, the kexec_load syscall fails. 

This week I posted additional IMA kselftests[1][2], including one for
the kexec_file_load syscall.  I would really appreciate these
kselftests being reviewed/acked.

Mimi

[1] Subject: [PATCH v2 0/5] selftests/ima: add kexec and kernel module tests
[2] Patches available from the "next-queued-testing" branch
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/
Dave Young March 8, 2019, 2:44 a.m. UTC | #4
On 02/28/19 at 10:05am, Mimi Zohar wrote:
> Hi Dave,
> 
> On Thu, 2019-02-28 at 21:41 +0800, Dave Young wrote:
> > Hi Mimi,
> >  
> > Sorry for jumping in late, just noticed this kexec selftests, I think we
> > also need a kexec load test not only for ima, but for general kexec
> 
> The IMA kselftest tests are for the coordination between the different
> methods of verifying file signatures.  In particular, for the kexec
> kernel image and kernel module signatures.
> 
> The initial IMA kselftest just verifies that in an environment
> requiring signed kexec kernel images, the kexec_load syscall fails. 
> 
> This week I posted additional IMA kselftests[1][2], including one for
> the kexec_file_load syscall.  I would really appreciate these
> kselftests being reviewed/acked.
> 
> Mimi
> 
> [1] Subject: [PATCH v2 0/5] selftests/ima: add kexec and kernel module tests
> [2] Patches available from the "next-queued-testing" branch
> https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/
> 

Hi Mimi,

Still did not get change to have a look at V2,  but seems you missed the
last chunk of comments about the secure boot mode in previous reply?

I just copy it hear:
'''
Do you want to get the Secureboot status here?
I got some advice from Peter Jones previously, thus we have below
in our kdump scripts:
https://src.fedoraproject.org/cgit/rpms/kexec-tools.git/tree/kdump-lib.sh

See the function is_secure_boot_enforced(), probably you can refer to
that function and check setup mode as well.
'''

Thanks
Dave
Mimi Zohar March 8, 2019, 1:45 p.m. UTC | #5
On Fri, 2019-03-08 at 10:44 +0800, Dave Young wrote:
> Hi Mimi,
> 
> Still did not get change to have a look at V2,  but seems you missed the
> last chunk of comments about the secure boot mode in previous reply?
> 
> I just copy it hear:
> '''
> Do you want to get the Secureboot status here?
> I got some advice from Peter Jones previously, thus we have below
> in our kdump scripts:
> https://src.fedoraproject.org/cgit/rpms/kexec-tools.git/tree/kdump-lib.sh
> 
> See the function is_secure_boot_enforced(), probably you can refer to
> that function and check setup mode as well.
> '''

Thank you for the pointer to the kdump scripts and reminder.

Mimi

Patch
diff mbox series

diff --git a/tools/testing/selftests/ima/common_lib.sh b/tools/testing/selftests/ima/common_lib.sh
new file mode 100755
index 000000000000..ae097a634da5
--- /dev/null
+++ b/tools/testing/selftests/ima/common_lib.sh
@@ -0,0 +1,20 @@ 
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+
+get_secureboot_mode()
+{
+	EFIVARFS="/sys/firmware/efi/efivars"
+	# 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
+	return `hexdump $file | awk '{print substr($4,length($4),1)}'`
+}
diff --git a/tools/testing/selftests/ima/test_kexec_load.sh b/tools/testing/selftests/ima/test_kexec_load.sh
index 74423c4229e2..5e3566738888 100755
--- a/tools/testing/selftests/ima/test_kexec_load.sh
+++ b/tools/testing/selftests/ima/test_kexec_load.sh
@@ -5,7 +5,7 @@ 
 # is booted in secureboot mode.
 
 TEST="$0"
-EFIVARFS="/sys/firmware/efi/efivars"
+. ./common_lib.sh
 rc=0
 
 # Kselftest framework requirement - SKIP code is 4.
@@ -17,19 +17,8 @@  if [ $(id -ru) != 0 ]; then
 	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)}'`
+get_secureboot_mode
+secureboot=$?
 
 # kexec_load should fail in secure boot mode
 KERNEL_IMAGE="/boot/vmlinuz-`uname -r`"