diff mbox series

[v2] Add tst_secureboot_enabled() helper function

Message ID 20201109133245.10879-1-mdoucha@suse.cz
State Changes Requested
Headers show
Series [v2] Add tst_secureboot_enabled() helper function | expand

Commit Message

Martin Doucha Nov. 9, 2020, 1:32 p.m. UTC 
Also check for SecureBoot status in tst_lockdown_enabled() if the lockdown
sysfile is not available/readable

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

Changes since v1:
- check whether machine is in EFI mode first

The check was not strictly required, efi_get_variable() would simply fail
on non-EFI systems with error message "get_variable() not implemented". But
it's better to have a less confusing status message anyway.

 configure.ac            |  1 +
 include/mk/config.mk.in |  4 ++--
 include/tst_lockdown.h  |  1 +
 lib/tst_lockdown.c      | 49 +++++++++++++++++++++++++++++++++++++++++
 m4/ltp-libefivar.m4     |  9 ++++++++
 5 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100644 m4/ltp-libefivar.m4

Comments

Cyril Hrubis Nov. 9, 2020, 1:56 p.m. UTC | #1 
Hi!
> diff --git a/configure.ac b/configure.ac
> index 03e4e09c9..d9ca5ad38 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -296,6 +296,7 @@ LTP_CHECK_CAPABILITY_SUPPORT
>  LTP_CHECK_CC_WARN_OLDSTYLE
>  LTP_CHECK_CLONE_SUPPORTS_7_ARGS
>  LTP_CHECK_CRYPTO
> +LTP_CHECK_EFIVAR
>  LTP_CHECK_FORTIFY_SOURCE
>  LTP_CHECK_KERNEL_DEVEL
>  LTP_CHECK_KEYUTILS_SUPPORT
> diff --git a/include/mk/config.mk.in b/include/mk/config.mk.in
> index 427608a17..cffd11245 100644
> --- a/include/mk/config.mk.in
> +++ b/include/mk/config.mk.in
> @@ -56,8 +56,8 @@ libdir			:= @libdir@
>  mandir			:= @mandir@
>  
>  CPPFLAGS		:= @CPPFLAGS@
> -CFLAGS			:= @CFLAGS@
> -LDLIBS			:= @LIBS@
> +CFLAGS			:= @CFLAGS@ @EFIVAR_CFLAGS@
> +LDLIBS			:= @LIBS@ @EFIVAR_LIBS@

Please do not do this.

This should be handled like any other library, i.e. we should define
EFIVAR_CFLAGS and EFIVAR_LIBS and use them in respective Makefile.

>  LDFLAGS			:= @LDFLAGS@
>  
>  DEBUG_CFLAGS		?= -g
> diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
> index 78eaeccea..172a7daf5 100644
> --- a/include/tst_lockdown.h
> +++ b/include/tst_lockdown.h
> @@ -5,6 +5,7 @@
>  
>  #define PATH_LOCKDOWN	"/sys/kernel/security/lockdown"
>  
> +int tst_secureboot_enabled(void);
>  int tst_lockdown_enabled(void);
>  
>  #endif /* TST_LOCKDOWN_H */
> diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
> index e7c19813c..0a8d68ede 100644
> --- a/lib/tst_lockdown.c
> +++ b/lib/tst_lockdown.c
> @@ -2,21 +2,70 @@
>  
>  #define TST_NO_DEFAULT_MAIN
>  
> +#include "config.h"
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <sys/mount.h>
>  
> +#ifdef HAVE_EFIVAR
> +#include <efivar.h>
> +#endif /* HAVE_EFIVAR */
> +
>  #include "tst_test.h"
>  #include "tst_safe_macros.h"
>  #include "tst_safe_stdio.h"
>  #include "tst_lockdown.h"
>  
> +int tst_secureboot_enabled(void)
> +{
> +#ifdef HAVE_EFIVAR
> +	int ret, status = 0;
> +	uint8_t *data = NULL;
> +	size_t size = 0;
> +	uint32_t attrs = 0;
> +
> +	if (!efi_variables_supported()) {
> +		tst_res(TINFO, "SecureBoot: off (non-EFI system)");
> +		return 0;
> +	}
> +
> +	efi_error_clear();
> +	ret = efi_get_variable(EFI_GLOBAL_GUID, "SecureBoot", &data, &size,
> +		&attrs);
> +
> +	if (ret) {
> +		char *fn, *func, *msg;
> +		int ln, err, i = 0;
> +
> +		while (efi_error_get(i++, &fn, &func, &ln, &msg, &err) > 0)
> +			tst_res(TINFO, "Efivar error: %s", msg);
> +
> +		efi_error_clear();
> +	} else if (data) {
> +		status = *data;
> +		tst_res(TINFO, "SecureBoot: %s", status ? "on" : "off");
> +	}
> +
> +	if (data)
> +		free(data);
> +
> +	return status;
> +#else /* HAVE_EFIVAR */
> +	tst_res(TINFO, "%s(): LTP was built without efivar support", __func__);
> +	return -1;
> +#endif /* HAVE_EFIVAR */
> +}
> +
>  int tst_lockdown_enabled(void)
>  {
>  	char line[BUFSIZ];
>  	FILE *file;
>  
>  	if (access(PATH_LOCKDOWN, F_OK) != 0) {
> +		/* SecureBoot enabled means integrity lockdown */
> +		if (tst_secureboot_enabled() > 0)
> +			return 1;
> +
>  		tst_res(TINFO, "Unable to determine system lockdown state");
>  		return 0;
>  	}

And this should be put into a separate library if it requires linking
againts efivars library.

> diff --git a/m4/ltp-libefivar.m4 b/m4/ltp-libefivar.m4
> new file mode 100644
> index 000000000..0a2750701
> --- /dev/null
> +++ b/m4/ltp-libefivar.m4
> @@ -0,0 +1,9 @@
> +dnl SPDX-License-Identifier: GPL-2.0-or-later
> +dnl Copyright (c) 2020 SUSE LLC <mdoucha@suse.cz>
> +
> +AC_DEFUN([LTP_CHECK_EFIVAR], [
> +	dnl efivar library and headers
> +	PKG_CHECK_MODULES([EFIVAR], [efivar], [
> +		AC_DEFINE([HAVE_EFIVAR], [1], [Define to 1 if you have libefivar library and headers])
> +	], [have_efivar=no])
> +])
> -- 
> 2.28.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
Martin Doucha Nov. 9, 2020, 2:10 p.m. UTC | #2 
On 09. 11. 20 14:56, Cyril Hrubis wrote:
> Hi!
>> diff --git a/configure.ac b/configure.ac
>> index 03e4e09c9..d9ca5ad38 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -296,6 +296,7 @@ LTP_CHECK_CAPABILITY_SUPPORT
>>  LTP_CHECK_CC_WARN_OLDSTYLE
>>  LTP_CHECK_CLONE_SUPPORTS_7_ARGS
>>  LTP_CHECK_CRYPTO
>> +LTP_CHECK_EFIVAR
>>  LTP_CHECK_FORTIFY_SOURCE
>>  LTP_CHECK_KERNEL_DEVEL
>>  LTP_CHECK_KEYUTILS_SUPPORT
>> diff --git a/include/mk/config.mk.in b/include/mk/config.mk.in
>> index 427608a17..cffd11245 100644
>> --- a/include/mk/config.mk.in
>> +++ b/include/mk/config.mk.in
>> @@ -56,8 +56,8 @@ libdir			:= @libdir@
>>  mandir			:= @mandir@
>>  
>>  CPPFLAGS		:= @CPPFLAGS@
>> -CFLAGS			:= @CFLAGS@
>> -LDLIBS			:= @LIBS@
>> +CFLAGS			:= @CFLAGS@ @EFIVAR_CFLAGS@
>> +LDLIBS			:= @LIBS@ @EFIVAR_LIBS@
> 
> Please do not do this.
> 
> This should be handled like any other library, i.e. we should define
> EFIVAR_CFLAGS and EFIVAR_LIBS and use them in respective Makefile.

OK. Could you add some explanation of the policy for library
dependencies to the build system docs later? Thanks.
Petr Vorel Nov. 9, 2020, 3:15 p.m. UTC | #3 
Hi Martin, Cyril,

...
> >> diff --git a/include/mk/config.mk.in b/include/mk/config.mk.in
> >> index 427608a17..cffd11245 100644
> >> --- a/include/mk/config.mk.in
> >> +++ b/include/mk/config.mk.in
> >> @@ -56,8 +56,8 @@ libdir			:= @libdir@
> >>  mandir			:= @mandir@

> >>  CPPFLAGS		:= @CPPFLAGS@
> >> -CFLAGS			:= @CFLAGS@
> >> -LDLIBS			:= @LIBS@
> >> +CFLAGS			:= @CFLAGS@ @EFIVAR_CFLAGS@
> >> +LDLIBS			:= @LIBS@ @EFIVAR_LIBS@

> > Please do not do this.

> > This should be handled like any other library, i.e. we should define
> > EFIVAR_CFLAGS and EFIVAR_LIBS and use them in respective Makefile.
+1 (and sorry for overlooking this).

Kind regards,
Petr
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 03e4e09c9..d9ca5ad38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -296,6 +296,7 @@  LTP_CHECK_CAPABILITY_SUPPORT
 LTP_CHECK_CC_WARN_OLDSTYLE
 LTP_CHECK_CLONE_SUPPORTS_7_ARGS
 LTP_CHECK_CRYPTO
+LTP_CHECK_EFIVAR
 LTP_CHECK_FORTIFY_SOURCE
 LTP_CHECK_KERNEL_DEVEL
 LTP_CHECK_KEYUTILS_SUPPORT
diff --git a/include/mk/config.mk.in b/include/mk/config.mk.in
index 427608a17..cffd11245 100644
--- a/include/mk/config.mk.in
+++ b/include/mk/config.mk.in
@@ -56,8 +56,8 @@  libdir			:= @libdir@
 mandir			:= @mandir@
 
 CPPFLAGS		:= @CPPFLAGS@
-CFLAGS			:= @CFLAGS@
-LDLIBS			:= @LIBS@
+CFLAGS			:= @CFLAGS@ @EFIVAR_CFLAGS@
+LDLIBS			:= @LIBS@ @EFIVAR_LIBS@
 LDFLAGS			:= @LDFLAGS@
 
 DEBUG_CFLAGS		?= -g
diff --git a/include/tst_lockdown.h b/include/tst_lockdown.h
index 78eaeccea..172a7daf5 100644
--- a/include/tst_lockdown.h
+++ b/include/tst_lockdown.h
@@ -5,6 +5,7 @@ 
 
 #define PATH_LOCKDOWN	"/sys/kernel/security/lockdown"
 
+int tst_secureboot_enabled(void);
 int tst_lockdown_enabled(void);
 
 #endif /* TST_LOCKDOWN_H */
diff --git a/lib/tst_lockdown.c b/lib/tst_lockdown.c
index e7c19813c..0a8d68ede 100644
--- a/lib/tst_lockdown.c
+++ b/lib/tst_lockdown.c
@@ -2,21 +2,70 @@ 
 
 #define TST_NO_DEFAULT_MAIN
 
+#include "config.h"
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/mount.h>
 
+#ifdef HAVE_EFIVAR
+#include <efivar.h>
+#endif /* HAVE_EFIVAR */
+
 #include "tst_test.h"
 #include "tst_safe_macros.h"
 #include "tst_safe_stdio.h"
 #include "tst_lockdown.h"
 
+int tst_secureboot_enabled(void)
+{
+#ifdef HAVE_EFIVAR
+	int ret, status = 0;
+	uint8_t *data = NULL;
+	size_t size = 0;
+	uint32_t attrs = 0;
+
+	if (!efi_variables_supported()) {
+		tst_res(TINFO, "SecureBoot: off (non-EFI system)");
+		return 0;
+	}
+
+	efi_error_clear();
+	ret = efi_get_variable(EFI_GLOBAL_GUID, "SecureBoot", &data, &size,
+		&attrs);
+
+	if (ret) {
+		char *fn, *func, *msg;
+		int ln, err, i = 0;
+
+		while (efi_error_get(i++, &fn, &func, &ln, &msg, &err) > 0)
+			tst_res(TINFO, "Efivar error: %s", msg);
+
+		efi_error_clear();
+	} else if (data) {
+		status = *data;
+		tst_res(TINFO, "SecureBoot: %s", status ? "on" : "off");
+	}
+
+	if (data)
+		free(data);
+
+	return status;
+#else /* HAVE_EFIVAR */
+	tst_res(TINFO, "%s(): LTP was built without efivar support", __func__);
+	return -1;
+#endif /* HAVE_EFIVAR */
+}
+
 int tst_lockdown_enabled(void)
 {
 	char line[BUFSIZ];
 	FILE *file;
 
 	if (access(PATH_LOCKDOWN, F_OK) != 0) {
+		/* SecureBoot enabled means integrity lockdown */
+		if (tst_secureboot_enabled() > 0)
+			return 1;
+
 		tst_res(TINFO, "Unable to determine system lockdown state");
 		return 0;
 	}
diff --git a/m4/ltp-libefivar.m4 b/m4/ltp-libefivar.m4
new file mode 100644
index 000000000..0a2750701
--- /dev/null
+++ b/m4/ltp-libefivar.m4
@@ -0,0 +1,9 @@ 
+dnl SPDX-License-Identifier: GPL-2.0-or-later
+dnl Copyright (c) 2020 SUSE LLC <mdoucha@suse.cz>
+
+AC_DEFUN([LTP_CHECK_EFIVAR], [
+	dnl efivar library and headers
+	PKG_CHECK_MODULES([EFIVAR], [efivar], [
+		AC_DEFINE([HAVE_EFIVAR], [1], [Define to 1 if you have libefivar library and headers])
+	], [have_efivar=no])
+])