| Message ID | 20230720150206.1338520-4-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add .skip_in_secureboot flag | expand |
Hi, On 20. 07. 23 17:02, Petr Vorel wrote: > Enabled SecureBoot requires signed modules (regardless lockdown state). > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > .../syscalls/delete_module/delete_module01.c | 2 ++ > .../syscalls/delete_module/delete_module03.c | 2 ++ > .../syscalls/finit_module/finit_module01.c | 2 ++ > .../syscalls/finit_module/finit_module02.c | 17 +++++++++++++---- > .../kernel/syscalls/init_module/init_module01.c | 2 ++ > .../kernel/syscalls/init_module/init_module02.c | 16 ++++++++++++---- > 6 files changed, 33 insertions(+), 8 deletions(-) > > diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c > index 6ecd2cad1..08597cfd6 100644 > --- a/testcases/kernel/syscalls/delete_module/delete_module01.c > +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c > @@ -52,6 +52,8 @@ static struct tst_test test = { > .needs_root = 1, > /* lockdown requires signed modules */ > .skip_in_lockdown = 1, > + /* SecureBoot requires signed modules */ Nit: the two comments could be merged into one. > + .skip_in_secureboot = 1, > .cleanup = cleanup, > .test_all = do_delete_module, > }; > diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c > index 863d36188..a4b5108f0 100644 > --- a/testcases/kernel/syscalls/delete_module/delete_module03.c > +++ b/testcases/kernel/syscalls/delete_module/delete_module03.c > @@ -74,6 +74,8 @@ static struct tst_test test = { > .needs_root = 1, > /* lockdown requires signed modules */ > .skip_in_lockdown = 1, > + /* SecureBoot requires signed modules */ > + .skip_in_secureboot = 1, > .setup = setup, > .cleanup = cleanup, > .test_all = do_delete_module, > diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c > index f960b2e40..660b567f5 100644 > --- a/testcases/kernel/syscalls/finit_module/finit_module01.c > +++ b/testcases/kernel/syscalls/finit_module/finit_module01.c > @@ -51,4 +51,6 @@ static struct tst_test test = { > .needs_root = 1, > /* lockdown requires signed modules */ > .skip_in_lockdown = 1, > + /* SecureBoot requires signed modules */ > + .skip_in_secureboot = 1, > }; > diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c > index a7434de7d..4f5962829 100644 > --- a/testcases/kernel/syscalls/finit_module/finit_module02.c > +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c > @@ -25,7 +25,7 @@ > static char *mod_path; > > static int fd, fd_zero, fd_invalid = -1, fd_dir; > -static int kernel_lockdown; > +static int kernel_lockdown, secure_boot; > > 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); > @@ -84,6 +84,8 @@ static void setup(void) > tst_module_exists(MODULE_NAME, &mod_path); > > kernel_lockdown = tst_lockdown_enabled(); > + secure_boot = tst_secureboot_enabled(); > + > SAFE_MKDIR(TEST_DIR, 0700); > fd_dir = SAFE_OPEN(TEST_DIR, O_DIRECTORY); > > @@ -102,9 +104,16 @@ static void run(unsigned int n) > { > struct tcase *tc = &tcases[n]; > > - if (tc->skip_in_lockdown && kernel_lockdown) { > - tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); > - return; > + if (tc->skip_in_lockdown) { > + if (secure_boot) { > + tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name); > + return; > + } > + > + if (kernel_lockdown) { > + tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); > + return; > + } It'd be better to just change the original TCONF message to something like "Cannot load unsigned modules, skipping %s". Adding a TINFO message "Lockdown on/off" to tst_lockdown_enabled() would provide the explanation why. > } > > fd = SAFE_OPEN(mod_path, tc->open_flags); > diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c > index 79e567cd6..80b2b77cc 100644 > --- a/testcases/kernel/syscalls/init_module/init_module01.c > +++ b/testcases/kernel/syscalls/init_module/init_module01.c > @@ -55,4 +55,6 @@ static struct tst_test test = { > .needs_root = 1, > /* lockdown requires signed modules */ > .skip_in_lockdown = 1, > + /* SecureBoot requires signed modules */ > + .skip_in_secureboot = 1, > }; > diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c > index ad6569a06..4acbfbcd1 100644 > --- a/testcases/kernel/syscalls/init_module/init_module02.c > +++ b/testcases/kernel/syscalls/init_module/init_module02.c > @@ -22,7 +22,7 @@ > #define MODULE_NAME "init_module.ko" > > static unsigned long size, zero_size; > -static int kernel_lockdown; > +static int kernel_lockdown, secure_boot; > static void *buf, *faulty_buf, *null_buf; > > static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE); > @@ -54,6 +54,7 @@ static void setup(void) > tst_module_exists(MODULE_NAME, NULL); > > kernel_lockdown = tst_lockdown_enabled(); > + secure_boot = tst_secureboot_enabled(); > fd = SAFE_OPEN(MODULE_NAME, O_RDONLY|O_CLOEXEC); > SAFE_FSTAT(fd, &sb); > size = sb.st_size; > @@ -67,9 +68,16 @@ static void run(unsigned int n) > { > struct tcase *tc = &tcases[n]; > > - if (tc->skip_in_lockdown && kernel_lockdown) { > - tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); > - return; > + if (tc->skip_in_lockdown) { > + if (secure_boot) { > + tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name); > + return; > + } > + > + if (kernel_lockdown) { > + tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); > + return; > + } Same here. > } > > if (tc->cap)
Hi Martin, > > +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c > > @@ -52,6 +52,8 @@ static struct tst_test test = { > > .needs_root = 1, > > /* lockdown requires signed modules */ > > .skip_in_lockdown = 1, > > + /* SecureBoot requires signed modules */ > Nit: the two comments could be merged into one. +1 ... > > @@ -102,9 +104,16 @@ static void run(unsigned int n) > > { > > struct tcase *tc = &tcases[n]; > > - if (tc->skip_in_lockdown && kernel_lockdown) { > > - tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); > > - return; > > + if (tc->skip_in_lockdown) { > > + if (secure_boot) { > > + tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name); > > + return; > > + } > > + > > + if (kernel_lockdown) { > > + tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); > > + return; > > + } > It'd be better to just change the original TCONF message to something like > "Cannot load unsigned modules, skipping %s". Adding a TINFO message > "Lockdown on/off" to tst_lockdown_enabled() would provide the explanation > why. +1 Kind regards, Petr
diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c index 6ecd2cad1..08597cfd6 100644 --- a/testcases/kernel/syscalls/delete_module/delete_module01.c +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c @@ -52,6 +52,8 @@ static struct tst_test test = { .needs_root = 1, /* lockdown requires signed modules */ .skip_in_lockdown = 1, + /* SecureBoot requires signed modules */ + .skip_in_secureboot = 1, .cleanup = cleanup, .test_all = do_delete_module, }; diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c index 863d36188..a4b5108f0 100644 --- a/testcases/kernel/syscalls/delete_module/delete_module03.c +++ b/testcases/kernel/syscalls/delete_module/delete_module03.c @@ -74,6 +74,8 @@ static struct tst_test test = { .needs_root = 1, /* lockdown requires signed modules */ .skip_in_lockdown = 1, + /* SecureBoot requires signed modules */ + .skip_in_secureboot = 1, .setup = setup, .cleanup = cleanup, .test_all = do_delete_module, diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c index f960b2e40..660b567f5 100644 --- a/testcases/kernel/syscalls/finit_module/finit_module01.c +++ b/testcases/kernel/syscalls/finit_module/finit_module01.c @@ -51,4 +51,6 @@ static struct tst_test test = { .needs_root = 1, /* lockdown requires signed modules */ .skip_in_lockdown = 1, + /* SecureBoot requires signed modules */ + .skip_in_secureboot = 1, }; diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c index a7434de7d..4f5962829 100644 --- a/testcases/kernel/syscalls/finit_module/finit_module02.c +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c @@ -25,7 +25,7 @@ static char *mod_path; static int fd, fd_zero, fd_invalid = -1, fd_dir; -static int kernel_lockdown; +static int kernel_lockdown, secure_boot; 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); @@ -84,6 +84,8 @@ static void setup(void) tst_module_exists(MODULE_NAME, &mod_path); kernel_lockdown = tst_lockdown_enabled(); + secure_boot = tst_secureboot_enabled(); + SAFE_MKDIR(TEST_DIR, 0700); fd_dir = SAFE_OPEN(TEST_DIR, O_DIRECTORY); @@ -102,9 +104,16 @@ static void run(unsigned int n) { struct tcase *tc = &tcases[n]; - if (tc->skip_in_lockdown && kernel_lockdown) { - tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); - return; + if (tc->skip_in_lockdown) { + if (secure_boot) { + tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name); + return; + } + + if (kernel_lockdown) { + tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); + return; + } } fd = SAFE_OPEN(mod_path, tc->open_flags); diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c index 79e567cd6..80b2b77cc 100644 --- a/testcases/kernel/syscalls/init_module/init_module01.c +++ b/testcases/kernel/syscalls/init_module/init_module01.c @@ -55,4 +55,6 @@ static struct tst_test test = { .needs_root = 1, /* lockdown requires signed modules */ .skip_in_lockdown = 1, + /* SecureBoot requires signed modules */ + .skip_in_secureboot = 1, }; diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c index ad6569a06..4acbfbcd1 100644 --- a/testcases/kernel/syscalls/init_module/init_module02.c +++ b/testcases/kernel/syscalls/init_module/init_module02.c @@ -22,7 +22,7 @@ #define MODULE_NAME "init_module.ko" static unsigned long size, zero_size; -static int kernel_lockdown; +static int kernel_lockdown, secure_boot; static void *buf, *faulty_buf, *null_buf; static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE); @@ -54,6 +54,7 @@ static void setup(void) tst_module_exists(MODULE_NAME, NULL); kernel_lockdown = tst_lockdown_enabled(); + secure_boot = tst_secureboot_enabled(); fd = SAFE_OPEN(MODULE_NAME, O_RDONLY|O_CLOEXEC); SAFE_FSTAT(fd, &sb); size = sb.st_size; @@ -67,9 +68,16 @@ static void run(unsigned int n) { struct tcase *tc = &tcases[n]; - if (tc->skip_in_lockdown && kernel_lockdown) { - tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); - return; + if (tc->skip_in_lockdown) { + if (secure_boot) { + tst_res(TCONF, "SecureBoot enabled, skipping %s", tc->name); + return; + } + + if (kernel_lockdown) { + tst_res(TCONF, "Kernel is locked down, skipping %s", tc->name); + return; + } } if (tc->cap)
Enabled SecureBoot requires signed modules (regardless lockdown state). Signed-off-by: Petr Vorel <pvorel@suse.cz> --- .../syscalls/delete_module/delete_module01.c | 2 ++ .../syscalls/delete_module/delete_module03.c | 2 ++ .../syscalls/finit_module/finit_module01.c | 2 ++ .../syscalls/finit_module/finit_module02.c | 17 +++++++++++++---- .../kernel/syscalls/init_module/init_module01.c | 2 ++ .../kernel/syscalls/init_module/init_module02.c | 16 ++++++++++++---- 6 files changed, 33 insertions(+), 8 deletions(-)