diff mbox series

[2/3] init_module: To handle kernel module signature enforcement

Message ID 20240308045230.3106549-2-liwang@redhat.com
State Superseded
Headers show
Series [1/3] kconfig: add funtion to parse /proc/cmdline | expand

Commit Message

Li Wang March 8, 2024, 4:52 a.m. UTC 
The patch modifies init_module syscall test cases to account
for kernel module signature enforcement. It adds parsing for
the 'module.sig_enforce' parameter and adjusts test expectations
based on whether signature enforcement is enabled, using
new conditional logic.

If enforcement is active, tests expect an EKEYREJECTED error;
otherwise, they proceed as normal.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 .../syscalls/delete_module/delete_module01.c  |  8 ++++
 .../syscalls/delete_module/delete_module03.c  |  8 ++++
 .../syscalls/finit_module/finit_module01.c    | 15 +++++++-
 .../syscalls/finit_module/finit_module02.c    | 37 ++++++++++++-------
 .../syscalls/init_module/init_module01.c      | 13 +++++++
 .../syscalls/init_module/init_module02.c      | 19 +++++++++-
 6 files changed, 85 insertions(+), 15 deletions(-)

Comments

Petr Vorel March 8, 2024, 8:47 a.m. UTC | #1 
Hi Li,

> The patch modifies init_module syscall test cases to account
> for kernel module signature enforcement. It adds parsing for
> the 'module.sig_enforce' parameter and adjusts test expectations
> based on whether signature enforcement is enabled, using
> new conditional logic.

> If enforcement is active, tests expect an EKEYREJECTED error;
> otherwise, they proceed as normal.

...
> diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
> index 90d8b5289..4c31f81b0 100644
> --- a/testcases/kernel/syscalls/delete_module/delete_module01.c
> +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> @@ -14,8 +14,10 @@
>   * Install dummy_del_mod.ko and delete it with delete_module(2).
>   */

> +#include <stdlib.h>
>  #include "tst_test.h"
>  #include "tst_module.h"
> +#include "tst_kconfig.h"
>  #include "lapi/syscalls.h"

>  #define MODULE_NAME	"dummy_del_mod"
> @@ -25,6 +27,12 @@ static int module_loaded;

>  static void do_delete_module(void)
>  {
> +	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
> +
> +	tst_kcmdline_parse(&params, 1);
> +	if (atoi(params.value) == 1)
> +		tst_brk(TCONF, "module signature is enforced, skip test");
Only 2 tests do tst_brk(TCONF). I was thinking about adding library flag
.skip_in_module_sig_enforce, but probably not worth of it.

The rest LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
Li Wang March 9, 2024, 8:11 a.m. UTC | #2 
On Fri, Mar 8, 2024 at 4:47 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > The patch modifies init_module syscall test cases to account
> > for kernel module signature enforcement. It adds parsing for
> > the 'module.sig_enforce' parameter and adjusts test expectations
> > based on whether signature enforcement is enabled, using
> > new conditional logic.
>
> > If enforcement is active, tests expect an EKEYREJECTED error;
> > otherwise, they proceed as normal.
>
> ...
> > diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c
> b/testcases/kernel/syscalls/delete_module/delete_module01.c
> > index 90d8b5289..4c31f81b0 100644
> > --- a/testcases/kernel/syscalls/delete_module/delete_module01.c
> > +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> > @@ -14,8 +14,10 @@
> >   * Install dummy_del_mod.ko and delete it with delete_module(2).
> >   */
>
> > +#include <stdlib.h>
> >  #include "tst_test.h"
> >  #include "tst_module.h"
> > +#include "tst_kconfig.h"
> >  #include "lapi/syscalls.h"
>
> >  #define MODULE_NAME  "dummy_del_mod"
> > @@ -25,6 +27,12 @@ static int module_loaded;
>
> >  static void do_delete_module(void)
> >  {
> > +     struct tst_kcmdline_param params =
> TST_KCMDLINE_INIT("module.sig_enforce");
> > +
> > +     tst_kcmdline_parse(&params, 1);
> > +     if (atoi(params.value) == 1)
> > +             tst_brk(TCONF, "module signature is enforced, skip test");
> Only 2 tests do tst_brk(TCONF). I was thinking about adding library flag
> .skip_in_module_sig_enforce, but probably not worth of it.
>

Hmm, it is not very necessary at this moment.
Maybe in the future, it could be an extended direction.



>
> The rest LGTM.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
>

Thanks a lot for testing/reviewing, the reset comments look good to me.

V2 is coming.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
index 90d8b5289..4c31f81b0 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module01.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
@@ -14,8 +14,10 @@ 
  * Install dummy_del_mod.ko and delete it with delete_module(2).
  */
 
+#include <stdlib.h>
 #include "tst_test.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 #include "lapi/syscalls.h"
 
 #define MODULE_NAME	"dummy_del_mod"
@@ -25,6 +27,12 @@  static int module_loaded;
 
 static void do_delete_module(void)
 {
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	if (atoi(params.value) == 1)
+		tst_brk(TCONF, "module signature is enforced, skip test");
+
 	if (!module_loaded) {
 		tst_module_load(MODULE_NAME_KO, NULL);
 		module_loaded = 1;
diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c
index 7e92fc2af..66a1fb815 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module03.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
@@ -12,9 +12,11 @@ 
  * if tried to remove a module while other modules depend on this module.
  */
 
+#include <stdlib.h>
 #include <errno.h>
 #include "tst_test.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 #include "lapi/syscalls.h"
 
 #define DUMMY_MOD		"dummy_del_mod"
@@ -50,6 +52,12 @@  static void do_delete_module(void)
 
 static void setup(void)
 {
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	if (atoi(params.value) == 1)
+		tst_brk(TCONF, "module signature is enforced, skip test");
+
 	/* Load first kernel module */
 	tst_module_load(DUMMY_MOD_KO, NULL);
 	dummy_mod_loaded = 1;
diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
index 1929c30fa..88d84297f 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module01.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
@@ -13,18 +13,25 @@ 
  * Inserts a simple module after opening and mmaping the module file.
  */
 
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 
 #define MODULE_NAME	"finit_module.ko"
 
-static int fd;
+static int fd, sig_enforce;
 
 static char *mod_path;
 
 static void setup(void)
 {
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
+
 	tst_module_exists(MODULE_NAME, &mod_path);
 
 	fd = SAFE_OPEN(mod_path, O_RDONLY|O_CLOEXEC);
@@ -32,6 +39,12 @@  static void setup(void)
 
 static void run(void)
 {
+	if (sig_enforce == 1) {
+		tst_res(TINFO, "module signature is enforced");
+		TST_EXP_FAIL(finit_module(fd, "status=valid", 0), EKEYREJECTED);
+		return;
+	}
+
 	TST_EXP_PASS(finit_module(fd, "status=valid", 0));
 	if (!TST_PASS)
 		return;
diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
index 223d9b388..0e9a787f0 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module02.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
@@ -14,9 +14,11 @@ 
  */
 
 #include <linux/capability.h>
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 #include "tst_capability.h"
 
 #define MODULE_NAME	"finit_module.ko"
@@ -25,7 +27,7 @@ 
 static char *mod_path;
 
 static int fd, fd_zero, fd_invalid = -1, fd_dir;
-static int kernel_lockdown, secure_boot;
+static int kernel_lockdown, secure_boot, sig_enforce;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
 static struct tst_cap cap_drop = TST_CAP(TST_CAP_DROP, CAP_SYS_MODULE);
@@ -59,27 +61,26 @@  static void dir_setup(struct tcase *tc)
 }
 
 static struct tcase tcases[] = {
-	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0,
-		bad_fd_setup},
+	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0, bad_fd_setup},
 	{"zero-fd", &fd_zero, "", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, 0, NULL},
 	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT, 1, NULL},
-	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0,
-		EINVAL, 1, NULL},
-	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, 0,
-		NULL},
+	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, 1, NULL},
+	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, 0, NULL},
 	{"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM, 0, NULL},
-	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, 1,
-		NULL},
-	{"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, EBADF, 0,
-		NULL},
-	{"file-readwrite", &fd, "", O_RDWR | O_CLOEXEC, 0, 0, ETXTBSY, 0,
-		NULL},
+	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, 1, NULL},
+	{"module-unsigned", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EKEYREJECTED, 1, NULL},
+	{"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, EBADF, 0, NULL},
+	{"file-readwrite", &fd, "", O_RDWR | O_CLOEXEC, 0, 0, ETXTBSY, 0, NULL},
 	{"directory", &fd_dir, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0, dir_setup},
 };
 
 static void setup(void)
 {
 	unsigned long int i;
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
 
 	tst_module_exists(MODULE_NAME, &mod_path);
 
@@ -109,6 +110,16 @@  static void run(unsigned int n)
 		return;
 	}
 
+	if ((sig_enforce == 1) && (tc->exp_errno != EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is enforced, skipping %s", tc->name);
+		return;
+	}
+
+	if ((sig_enforce != 1) && (tc->exp_errno == EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is not enforced, skipping %s", tc->name);
+		return;
+	}
+
 	fd = SAFE_OPEN(mod_path, tc->open_flags);
 
 	if (tc->cap)
diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
index 26ff0b93b..deba47571 100644
--- a/testcases/kernel/syscalls/init_module/init_module01.c
+++ b/testcases/kernel/syscalls/init_module/init_module01.c
@@ -13,18 +13,25 @@ 
  * Inserts a simple module after opening and mmaping the module file.
  */
 
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 
 #define MODULE_NAME	"init_module.ko"
 
 static struct stat sb;
 static void *buf;
+static int sig_enforce;
 
 static void setup(void)
 {
 	int fd;
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
 
 	tst_module_exists(MODULE_NAME, NULL);
 
@@ -36,6 +43,12 @@  static void setup(void)
 
 static void run(void)
 {
+	if (sig_enforce == 1) {
+		tst_res(TINFO, "module signature is enforced");
+		TST_EXP_FAIL(init_module(buf, sb.st_size, "status=valid"), EKEYREJECTED);
+		return;
+	}
+
 	TST_EXP_PASS(init_module(buf, sb.st_size, "status=valid"));
 	if (!TST_PASS)
 		return;
diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
index e6730e21c..a913c0b4c 100644
--- a/testcases/kernel/syscalls/init_module/init_module02.c
+++ b/testcases/kernel/syscalls/init_module/init_module02.c
@@ -14,15 +14,17 @@ 
  */
 
 #include <linux/capability.h>
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
+#include "tst_kconfig.h"
 #include "tst_module.h"
 #include "tst_capability.h"
 
 #define MODULE_NAME	"init_module.ko"
 
 static unsigned long size, zero_size;
-static int kernel_lockdown, secure_boot;
+static int kernel_lockdown, secure_boot, sig_enforce;
 static void *buf, *faulty_buf, *null_buf;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
@@ -44,12 +46,17 @@  static struct tcase {
 	{"invalid_param", &buf, &size, "status=invalid", 0, 1, EINVAL},
 	{"no-perm", &buf, &size, "", 1, 0, EPERM},
 	{"module-exists", &buf, &size, "", 0, 1, EEXIST},
+	{"module-unsigned", &buf, &size, "", 0, 1, EKEYREJECTED},
 };
 
 static void setup(void)
 {
 	struct stat sb;
 	int fd;
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
 
 	tst_module_exists(MODULE_NAME, NULL);
 
@@ -73,6 +80,16 @@  static void run(unsigned int n)
 		return;
 	}
 
+	if ((sig_enforce == 1) && (tc->exp_errno != EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is enforced, skipping %s", tc->name);
+		return;
+	}
+
+	if ((sig_enforce != 1) && (tc->exp_errno == EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is not enforced, skipping %s", tc->name);
+		return;
+	}
+
 	if (tc->cap)
 		tst_cap_action(&cap_drop);