[V4] selinux-testsuite: Add kernel module tests
diff mbox series

Message ID 20191119113845.89951-1-richard_c_haines@btinternet.com
State Superseded
Headers show
Series
  • [V4] selinux-testsuite: Add kernel module tests
Related show

Commit Message

Richard Haines Nov. 19, 2019, 11:38 a.m. UTC
Test kernel module loading permissions.

Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
V2 Change:
Check permission denial module_load versus module_request by using a
test kernel module for each.
Note: Rawhide (with secnext kernel) adds built-in.a and built-in.a.cmd when
building modules, therefore added to Makefile and .gitignore.
V3 Changes:
As requested in [1] except policy change, coalesced type attributes instead.
V4 Change:
Change attributes initmoddoman and finitmoddomain to a single attribute of
kmoduledomain.

[1] https://lore.kernel.org/selinux/CAFqZXNtm_X+YssnX_3_5ThkVZY+9SBeQC5Qo78s+geSsBok8=Q@mail.gmail.com/

 policy/Makefile                           |   4 +
 policy/test_module_load.te                | 108 +++++++++++++++++++
 tests/Makefile                            |   4 +
 tests/module_load/.gitignore              |  11 ++
 tests/module_load/Makefile                |  12 +++
 tests/module_load/finit_load.c            |  94 +++++++++++++++++
 tests/module_load/init_load.c             | 123 ++++++++++++++++++++++
 tests/module_load/setest_module_load.c    |  18 ++++
 tests/module_load/setest_module_request.c |  22 ++++
 tests/module_load/test                    |  62 +++++++++++
 10 files changed, 458 insertions(+)
 create mode 100644 policy/test_module_load.te
 create mode 100644 tests/module_load/.gitignore
 create mode 100644 tests/module_load/Makefile
 create mode 100644 tests/module_load/finit_load.c
 create mode 100644 tests/module_load/init_load.c
 create mode 100644 tests/module_load/setest_module_load.c
 create mode 100644 tests/module_load/setest_module_request.c
 create mode 100755 tests/module_load/test

Comments

Ondrej Mosnacek Nov. 20, 2019, 11:58 a.m. UTC | #1
On Tue, Nov 19, 2019 at 12:38 PM Richard Haines
<richard_c_haines@btinternet.com> wrote:
>
> Test kernel module loading permissions.
>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>

Acked-by: Ondrej Mosnacek <omosnace@redhat.com>

> ---
> V2 Change:
> Check permission denial module_load versus module_request by using a
> test kernel module for each.
> Note: Rawhide (with secnext kernel) adds built-in.a and built-in.a.cmd when
> building modules, therefore added to Makefile and .gitignore.
> V3 Changes:
> As requested in [1] except policy change, coalesced type attributes instead.
> V4 Change:
> Change attributes initmoddoman and finitmoddomain to a single attribute of
> kmoduledomain.
>
> [1] https://lore.kernel.org/selinux/CAFqZXNtm_X+YssnX_3_5ThkVZY+9SBeQC5Qo78s+geSsBok8=Q@mail.gmail.com/
>
>  policy/Makefile                           |   4 +
>  policy/test_module_load.te                | 108 +++++++++++++++++++
>  tests/Makefile                            |   4 +
>  tests/module_load/.gitignore              |  11 ++
>  tests/module_load/Makefile                |  12 +++
>  tests/module_load/finit_load.c            |  94 +++++++++++++++++
>  tests/module_load/init_load.c             | 123 ++++++++++++++++++++++
>  tests/module_load/setest_module_load.c    |  18 ++++
>  tests/module_load/setest_module_request.c |  22 ++++
>  tests/module_load/test                    |  62 +++++++++++
>  10 files changed, 458 insertions(+)
>  create mode 100644 policy/test_module_load.te
>  create mode 100644 tests/module_load/.gitignore
>  create mode 100644 tests/module_load/Makefile
>  create mode 100644 tests/module_load/finit_load.c
>  create mode 100644 tests/module_load/init_load.c
>  create mode 100644 tests/module_load/setest_module_load.c
>  create mode 100644 tests/module_load/setest_module_request.c
>  create mode 100755 tests/module_load/test
>
> diff --git a/policy/Makefile b/policy/Makefile
> index ad94c43..25dfb69 100644
> --- a/policy/Makefile
> +++ b/policy/Makefile
> @@ -94,6 +94,10 @@ ifeq ($(shell grep -q key_socket $(POLDEV)/include/support/all_perms.spt && echo
>  TARGETS += test_key_socket.te
>  endif
>
> +ifeq ($(shell grep -q module_load $(POLDEV)/include/support/all_perms.spt && echo true),true)
> +TARGETS+=test_module_load.te
> +endif
> +
>  ifeq (x$(DISTRO),$(filter x$(DISTRO),xRHEL4 xRHEL5 xRHEL6))
>  TARGETS:=$(filter-out test_overlayfs.te test_mqueue.te test_ibpkey.te, $(TARGETS))
>  endif
> diff --git a/policy/test_module_load.te b/policy/test_module_load.te
> new file mode 100644
> index 0000000..5496d86
> --- /dev/null
> +++ b/policy/test_module_load.te
> @@ -0,0 +1,108 @@
> +############# Test kernel modules ###################
> +#
> +attribute kmoduledomain;
> +
> +#
> +############################## Define Macro ################################
> +#
> +# Replace domain_type() macro as it hides some relevant denials in audit.log
> +#
> +gen_require(`
> +       type setrans_var_run_t, syslogd_t;
> +')
> +
> +define(`module_domain_type',`
> +       allow $1 proc_t:dir { search };
> +       allow $1 proc_t:lnk_file { read };
> +       allow $1 self:dir { search };
> +       allow $1 self:file { open read write };
> +       dontaudit init_t syslogd_t:fd use;
> +       dontaudit $1 security_t:filesystem getattr;
> +       dontaudit $1 self:file getattr;
> +       dontaudit $1 setrans_var_run_t:dir search;
> +       dontaudit unconfined_t $1:process { noatsecure rlimitinh siginh };
> +')
> +
> +#
> +############# Test kernel modules with finitmod_module(2) ###################
> +#
> +type test_finitmod_t;
> +module_domain_type(test_finitmod_t)
> +unconfined_runs_test(test_finitmod_t)
> +typeattribute test_finitmod_t testdomain, kmoduledomain;
> +
> +allow test_finitmod_t self:capability { sys_module };
> +allow test_finitmod_t test_file_t:system { module_load };
> +allow test_finitmod_t kernel_t:system { module_request };
> +
> +############### Deny cap sys_module ######################
> +type test_finitmod_deny_sys_module_t;
> +module_domain_type(test_finitmod_deny_sys_module_t)
> +unconfined_runs_test(test_finitmod_deny_sys_module_t)
> +typeattribute test_finitmod_deny_sys_module_t testdomain, kmoduledomain;
> +
> +neverallow test_finitmod_deny_sys_module_t self:capability { sys_module };
> +
> +############### Deny sys module_load ######################
> +type test_finitmod_deny_module_load_t;
> +module_domain_type(test_finitmod_deny_module_load_t)
> +unconfined_runs_test(test_finitmod_deny_module_load_t)
> +typeattribute test_finitmod_deny_module_load_t testdomain, kmoduledomain;
> +
> +allow test_finitmod_deny_module_load_t self:capability { sys_module };
> +neverallow test_finitmod_deny_module_load_t test_file_t:system { module_load };
> +
> +############### Deny sys module_request ######################
> +type test_finitmod_deny_module_request_t;
> +module_domain_type(test_finitmod_deny_module_request_t)
> +unconfined_runs_test(test_finitmod_deny_module_request_t)
> +typeattribute test_finitmod_deny_module_request_t testdomain, kmoduledomain;
> +
> +allow test_finitmod_deny_module_request_t self:capability { sys_module };
> +allow test_finitmod_deny_module_request_t test_file_t:system { module_load };
> +neverallow test_finitmod_deny_module_request_t kernel_t:system { module_request };
> +
> +#
> +############# Test kernel modules with initmod_module(2) ###################
> +#
> +type test_initmod_t;
> +module_domain_type(test_initmod_t)
> +unconfined_runs_test(test_initmod_t)
> +typeattribute test_initmod_t testdomain, kmoduledomain;
> +
> +allow test_initmod_t self:capability { sys_module };
> +allow test_initmod_t self:system { module_load };
> +allow test_initmod_t kernel_t:system { module_request };
> +
> +############### Deny cap sys_module ######################
> +type test_initmod_deny_sys_module_t;
> +module_domain_type(test_initmod_deny_sys_module_t)
> +unconfined_runs_test(test_initmod_deny_sys_module_t)
> +typeattribute test_initmod_deny_sys_module_t testdomain, kmoduledomain;
> +
> +neverallow test_initmod_deny_sys_module_t self:capability { sys_module };
> +
> +############### Deny sys module_load ######################
> +type test_initmod_deny_module_load_t;
> +module_domain_type(test_initmod_deny_module_load_t)
> +unconfined_runs_test(test_initmod_deny_module_load_t)
> +typeattribute test_initmod_deny_module_load_t testdomain, kmoduledomain;
> +
> +allow test_initmod_deny_module_load_t self:capability { sys_module };
> +neverallow test_initmod_deny_module_load_t self:system { module_load };
> +
> +############### Deny sys module_request ######################
> +type test_initmod_deny_module_request_t;
> +module_domain_type(test_initmod_deny_module_request_t)
> +unconfined_runs_test(test_initmod_deny_module_request_t)
> +typeattribute test_initmod_deny_module_request_t testdomain, kmoduledomain;
> +
> +allow test_initmod_deny_module_request_t self:capability { sys_module };
> +allow test_initmod_deny_module_request_t self:system { module_load };
> +neverallow test_initmod_deny_module_request_t kernel_t:system { module_request };
> +
> +#
> +########### Allow these domains to be entered from sysadm domain ############
> +#
> +miscfiles_domain_entry_test_files(kmoduledomain)
> +userdom_sysadm_entry_spec_domtrans_to(kmoduledomain)
> diff --git a/tests/Makefile b/tests/Makefile
> index cca6648..0452887 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -72,6 +72,10 @@ ifeq ($(shell grep -q all_file_perms.*watch $(POLDEV)/include/support/all_perms.
>  SUBDIRS+=notify
>  endif
>
> +ifeq ($(shell grep -q module_load $(POLDEV)/include/support/all_perms.spt && echo true),true)
> +SUBDIRS+=module_load
> +endif
> +
>  ifeq ($(DISTRO),RHEL4)
>      SUBDIRS:=$(filter-out bounds dyntrace dyntrans inet_socket mmap nnp_nosuid overlay unix_socket, $(SUBDIRS))
>  endif
> diff --git a/tests/module_load/.gitignore b/tests/module_load/.gitignore
> new file mode 100644
> index 0000000..7fa5772
> --- /dev/null
> +++ b/tests/module_load/.gitignore
> @@ -0,0 +1,11 @@
> +finit_load
> +init_load
> +modules.order
> +Module.symvers
> +*.a
> +*.o
> +*.ko
> +*.cmd
> +*.mod
> +*.mod.c
> +.*.cmd
> diff --git a/tests/module_load/Makefile b/tests/module_load/Makefile
> new file mode 100644
> index 0000000..b6eba25
> --- /dev/null
> +++ b/tests/module_load/Makefile
> @@ -0,0 +1,12 @@
> +obj-m = setest_module_load.o setest_module_request.o
> +
> +TARGETS = finit_load init_load
> +LDLIBS += -lselinux
> +KDIR = /lib/modules/$(shell uname -r)/build
> +
> +all: $(TARGETS)
> +       $(MAKE) -C $(KDIR) M=$(PWD)
> +
> +clean:
> +       rm -f $(TARGETS)
> +       rm -f *.a *.o *.ko *.cmd *.mod *.mod.c .*.cmd Module.symvers modules.order
> diff --git a/tests/module_load/finit_load.c b/tests/module_load/finit_load.c
> new file mode 100644
> index 0000000..1c05d7b
> --- /dev/null
> +++ b/tests/module_load/finit_load.c
> @@ -0,0 +1,94 @@
> +#define _GNU_SOURCE 1
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <sys/syscall.h>
> +#include <selinux/selinux.h>
> +
> +static void print_usage(char *progfile_name)
> +{
> +       fprintf(stderr,
> +               "usage:  %s [-v] path name\n"
> +               "Where:\n\t"
> +               "-v    Print information.\n\t"
> +               "path  Kernel module build path.\n\t"
> +               "name  Name of kernel module to load.\n", progfile_name);
> +       exit(-1);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +       char *context, file_name[PATH_MAX];
> +       int opt, result, fd, s_errno;
> +       bool verbose = false;
> +
> +       while ((opt = getopt(argc, argv, "v")) != -1) {
> +               switch (opt) {
> +               case 'v':
> +                       verbose = true;
> +                       break;
> +               default:
> +                       print_usage(argv[0]);
> +               }
> +       }
> +
> +       if (optind >= argc)
> +               print_usage(argv[0]);
> +
> +       result = sprintf(file_name, "%s/%s.ko", argv[optind],
> +                        argv[optind + 1]);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed sprintf\n");
> +               exit(-1);
> +       }
> +
> +       fd = open(file_name, O_RDONLY);
> +       if (!fd) {
> +               fprintf(stderr, "Failed to open %s: %s\n",
> +                       file_name, strerror(errno));
> +               exit(-1);
> +       }
> +
> +       if (verbose) {
> +               result = getcon(&context);
> +               if (result < 0) {
> +                       fprintf(stderr, "Failed to obtain process context\n");
> +                       close(fd);
> +                       exit(-1);
> +               }
> +
> +               printf("Process context:\n\t%s\n", context);
> +               free(context);
> +       }
> +
> +       result = syscall(__NR_finit_module, fd, "", 0);
> +       s_errno = errno;
> +       close(fd);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed to load '%s' module: %s\n",
> +                       file_name, strerror(s_errno));
> +               /* Denying: sys_module=EPERM, module_load=EACCES */
> +               exit(s_errno);
> +       }
> +
> +       if (verbose)
> +               printf("Loaded kernel module:  %s\n", file_name);
> +
> +       result = syscall(__NR_delete_module, argv[optind + 1], 0);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed to delete '%s' module: %s\n",
> +                       argv[optind + 1], strerror(errno));
> +               exit(-1);
> +       }
> +
> +       if (verbose)
> +               printf("Deleted kernel module: %s\n", argv[optind + 1]);
> +
> +       return 0;
> +}
> diff --git a/tests/module_load/init_load.c b/tests/module_load/init_load.c
> new file mode 100644
> index 0000000..0422c19
> --- /dev/null
> +++ b/tests/module_load/init_load.c
> @@ -0,0 +1,123 @@
> +#define _GNU_SOURCE 1
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <fcntl.h>
> +#include <limits.h>
> +#include <sys/stat.h>
> +#include <sys/syscall.h>
> +#include <selinux/selinux.h>
> +
> +static void print_usage(char *progfile_name)
> +{
> +       fprintf(stderr,
> +               "usage:  %s [-v] path name\n"
> +               "Where:\n\t"
> +               "-v    Print information.\n\t"
> +               "path  Kernel module build path.\n\t"
> +               "name  Name of kernel module to load.\n", progfile_name);
> +       exit(-1);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +       char *context, file_name[PATH_MAX];
> +       int opt, result, fd, s_errno;
> +       bool verbose = false;
> +       void *elf_image;
> +       struct stat st;
> +
> +       while ((opt = getopt(argc, argv, "v")) != -1) {
> +               switch (opt) {
> +               case 'v':
> +                       verbose = true;
> +                       break;
> +               default:
> +                       print_usage(argv[0]);
> +               }
> +       }
> +
> +       if (optind >= argc)
> +               print_usage(argv[0]);
> +
> +       result = sprintf(file_name, "%s/%s.ko", argv[optind],
> +                        argv[optind + 1]);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed sprintf\n");
> +               exit(-1);
> +       }
> +
> +       fd = open(file_name, O_RDONLY);
> +       if (!fd) {
> +               fprintf(stderr, "Failed to open %s: %s\n",
> +                       file_name, strerror(errno));
> +               exit(-1);
> +       }
> +
> +       if (verbose) {
> +               result = getcon(&context);
> +               if (result < 0) {
> +                       fprintf(stderr, "Failed to obtain process context\n");
> +                       close(fd);
> +                       exit(-1);
> +               }
> +
> +               printf("Process context:\n\t%s\n", context);
> +               free(context);
> +       }
> +
> +       result = fstat(fd, &st);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed fstat on %s: %s\n",
> +                       file_name, strerror(errno));
> +               close(fd);
> +               exit(-1);
> +       }
> +
> +       elf_image = malloc(st.st_size);
> +       if (!elf_image) {
> +               fprintf(stderr, "Failed malloc on %s: %s\n",
> +                       file_name, strerror(errno));
> +               close(fd);
> +               exit(-1);
> +       }
> +
> +       result = read(fd, elf_image, st.st_size);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed read on %s: %s\n",
> +                       file_name, strerror(errno));
> +               close(fd);
> +               free(elf_image);
> +               exit(-1);
> +       }
> +       close(fd);
> +
> +       result = syscall(__NR_init_module, elf_image, st.st_size, "");
> +       s_errno = errno;
> +       free(elf_image);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed to load '%s' module: %s\n",
> +                       file_name, strerror(s_errno));
> +               /* Denying: sys_module=EPERM, module_load & request=EACCES */
> +               exit(s_errno);
> +       }
> +
> +       if (verbose)
> +               printf("Loaded kernel module:  %s\n", file_name);
> +
> +       result = syscall(__NR_delete_module, argv[optind + 1], 0);
> +       if (result < 0) {
> +               fprintf(stderr, "Failed to delete '%s' module: %s\n",
> +                       argv[optind + 1], strerror(errno));
> +               exit(-1);
> +       }
> +
> +       if (verbose)
> +               printf("Deleted kernel module: %s\n", argv[optind + 1]);
> +
> +       return 0;
> +}
> diff --git a/tests/module_load/setest_module_load.c b/tests/module_load/setest_module_load.c
> new file mode 100644
> index 0000000..0be7a26
> --- /dev/null
> +++ b/tests/module_load/setest_module_load.c
> @@ -0,0 +1,18 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +
> +static int __init setest_module_load_init(void)
> +{
> +       pr_info("INIT - setest_module_load\n");
> +       return 0;
> +}
> +
> +static void __exit setest_module_load_exit(void)
> +{
> +       pr_info("EXIT - setest_module_load\n");
> +}
> +
> +module_init(setest_module_load_init);
> +module_exit(setest_module_load_exit);
> +MODULE_LICENSE("GPL");
> diff --git a/tests/module_load/setest_module_request.c b/tests/module_load/setest_module_request.c
> new file mode 100644
> index 0000000..f79d4ef
> --- /dev/null
> +++ b/tests/module_load/setest_module_request.c
> @@ -0,0 +1,22 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +
> +static int __init setest_module_request_init(void)
> +{
> +       int result;
> +
> +       pr_info("INIT - setest_module_request\n");
> +       result = request_module_nowait("dummy-module");
> +       pr_info("request_module() returned: %d\n", result);
> +       return result;
> +}
> +
> +static void __exit setest_module_request_exit(void)
> +{
> +       pr_info("EXIT - setest_module_request\n");
> +}
> +
> +module_init(setest_module_request_init);
> +module_exit(setest_module_request_exit);
> +MODULE_LICENSE("GPL");
> diff --git a/tests/module_load/test b/tests/module_load/test
> new file mode 100755
> index 0000000..c3242fc
> --- /dev/null
> +++ b/tests/module_load/test
> @@ -0,0 +1,62 @@
> +#!/usr/bin/perl
> +use Test::More;
> +
> +BEGIN {
> +    $basedir = $0;
> +    $basedir =~ s|(.*)/[^/]*|$1|;
> +
> +    # allow info to be shown during tests
> +    $v = $ARGV[0];
> +    if ($v) {
> +        if ( $v ne "-v" ) {
> +            plan skip_all => "Invalid option (use -v)";
> +        }
> +    }
> +    else {
> +        $v = " ";
> +    }
> +
> +    plan tests => 8;
> +}
> +
> +print "Test finit_module(2)\n";
> +$result = system
> +"runcon -t test_finitmod_t $basedir/finit_load $v $basedir setest_module_request";
> +ok( $result eq 0 );
> +
> +# Deny capability { sys_module } - EPERM
> +$result = system
> +"runcon -t test_finitmod_deny_sys_module_t $basedir/finit_load $v $basedir setest_module_load 2>&1";
> +ok( $result >> 8 eq 1 );
> +
> +# Deny system { module_load } - EACCES
> +$result = system
> +"runcon -t test_finitmod_deny_module_load_t $basedir/finit_load $v $basedir setest_module_load 2>&1";
> +ok( $result >> 8 eq 13 );
> +
> +# Deny system { module_request } - EACCES
> +$result = system
> +"runcon -t test_finitmod_deny_module_request_t $basedir/finit_load $v $basedir setest_module_request 2>&1";
> +ok( $result >> 8 eq 13 );
> +
> +print "Test init_module(2)\n";
> +$result = system
> +"runcon -t test_initmod_t $basedir/init_load $v $basedir setest_module_request";
> +ok( $result eq 0 );
> +
> +# Deny capability { sys_module } - EPERM
> +$result = system
> +"runcon -t test_initmod_deny_sys_module_t $basedir/init_load $v $basedir setest_module_load 2>&1";
> +ok( $result >> 8 eq 1 );
> +
> +# Deny system { module_load } - EACCES
> +$result = system
> +"runcon -t test_initmod_deny_module_load_t $basedir/init_load $v $basedir setest_module_load 2>&1";
> +ok( $result >> 8 eq 13 );
> +
> +# Deny system { module_request } - EACCES
> +$result = system
> +"runcon -t test_initmod_deny_module_request_t $basedir/init_load $v $basedir setest_module_request 2>&1";
> +ok( $result >> 8 eq 13 );
> +
> +exit;
> --
> 2.23.0
>


--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
Ondrej Mosnacek Nov. 22, 2019, 10:40 a.m. UTC | #2
On Wed, Nov 20, 2019 at 12:58 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Tue, Nov 19, 2019 at 12:38 PM Richard Haines
> <richard_c_haines@btinternet.com> wrote:
> >
> > Test kernel module loading permissions.
> >
> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>
> Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> > ---
> > V2 Change:
> > Check permission denial module_load versus module_request by using a
> > test kernel module for each.
> > Note: Rawhide (with secnext kernel) adds built-in.a and built-in.a.cmd when
> > building modules, therefore added to Makefile and .gitignore.
> > V3 Changes:
> > As requested in [1] except policy change, coalesced type attributes instead.
> > V4 Change:
> > Change attributes initmoddoman and finitmoddomain to a single attribute of
> > kmoduledomain.
> >
> > [1] https://lore.kernel.org/selinux/CAFqZXNtm_X+YssnX_3_5ThkVZY+9SBeQC5Qo78s+geSsBok8=Q@mail.gmail.com/
> >
> >  policy/Makefile                           |   4 +
> >  policy/test_module_load.te                | 108 +++++++++++++++++++
> >  tests/Makefile                            |   4 +
> >  tests/module_load/.gitignore              |  11 ++
> >  tests/module_load/Makefile                |  12 +++
> >  tests/module_load/finit_load.c            |  94 +++++++++++++++++
> >  tests/module_load/init_load.c             | 123 ++++++++++++++++++++++
> >  tests/module_load/setest_module_load.c    |  18 ++++
> >  tests/module_load/setest_module_request.c |  22 ++++
> >  tests/module_load/test                    |  62 +++++++++++
> >  10 files changed, 458 insertions(+)
> >  create mode 100644 policy/test_module_load.te
> >  create mode 100644 tests/module_load/.gitignore
> >  create mode 100644 tests/module_load/Makefile
> >  create mode 100644 tests/module_load/finit_load.c
> >  create mode 100644 tests/module_load/init_load.c
> >  create mode 100644 tests/module_load/setest_module_load.c
> >  create mode 100644 tests/module_load/setest_module_request.c
> >  create mode 100755 tests/module_load/test
> >
> > diff --git a/policy/Makefile b/policy/Makefile
> > index ad94c43..25dfb69 100644
> > --- a/policy/Makefile
> > +++ b/policy/Makefile
> > @@ -94,6 +94,10 @@ ifeq ($(shell grep -q key_socket $(POLDEV)/include/support/all_perms.spt && echo
> >  TARGETS += test_key_socket.te
> >  endif
> >
> > +ifeq ($(shell grep -q module_load $(POLDEV)/include/support/all_perms.spt && echo true),true)
> > +TARGETS+=test_module_load.te
> > +endif
> > +
> >  ifeq (x$(DISTRO),$(filter x$(DISTRO),xRHEL4 xRHEL5 xRHEL6))
> >  TARGETS:=$(filter-out test_overlayfs.te test_mqueue.te test_ibpkey.te, $(TARGETS))
> >  endif
> > diff --git a/policy/test_module_load.te b/policy/test_module_load.te
> > new file mode 100644
> > index 0000000..5496d86
> > --- /dev/null
> > +++ b/policy/test_module_load.te
> > @@ -0,0 +1,108 @@
> > +############# Test kernel modules ###################
> > +#
> > +attribute kmoduledomain;
> > +
> > +#
> > +############################## Define Macro ################################
> > +#
> > +# Replace domain_type() macro as it hides some relevant denials in audit.log
> > +#
> > +gen_require(`
> > +       type setrans_var_run_t, syslogd_t;
> > +')
> > +
> > +define(`module_domain_type',`
> > +       allow $1 proc_t:dir { search };
> > +       allow $1 proc_t:lnk_file { read };
> > +       allow $1 self:dir { search };
> > +       allow $1 self:file { open read write };
> > +       dontaudit init_t syslogd_t:fd use;
> > +       dontaudit $1 security_t:filesystem getattr;
> > +       dontaudit $1 self:file getattr;
> > +       dontaudit $1 setrans_var_run_t:dir search;
> > +       dontaudit unconfined_t $1:process { noatsecure rlimitinh siginh };
> > +')
> > +
> > +#
> > +############# Test kernel modules with finitmod_module(2) ###################
> > +#
> > +type test_finitmod_t;
> > +module_domain_type(test_finitmod_t)
> > +unconfined_runs_test(test_finitmod_t)
> > +typeattribute test_finitmod_t testdomain, kmoduledomain;
> > +
> > +allow test_finitmod_t self:capability { sys_module };
> > +allow test_finitmod_t test_file_t:system { module_load };
> > +allow test_finitmod_t kernel_t:system { module_request };
> > +
> > +############### Deny cap sys_module ######################
> > +type test_finitmod_deny_sys_module_t;
> > +module_domain_type(test_finitmod_deny_sys_module_t)
> > +unconfined_runs_test(test_finitmod_deny_sys_module_t)
> > +typeattribute test_finitmod_deny_sys_module_t testdomain, kmoduledomain;
> > +
> > +neverallow test_finitmod_deny_sys_module_t self:capability { sys_module };
> > +
> > +############### Deny sys module_load ######################
> > +type test_finitmod_deny_module_load_t;
> > +module_domain_type(test_finitmod_deny_module_load_t)
> > +unconfined_runs_test(test_finitmod_deny_module_load_t)
> > +typeattribute test_finitmod_deny_module_load_t testdomain, kmoduledomain;
> > +
> > +allow test_finitmod_deny_module_load_t self:capability { sys_module };
> > +neverallow test_finitmod_deny_module_load_t test_file_t:system { module_load };
> > +
> > +############### Deny sys module_request ######################
> > +type test_finitmod_deny_module_request_t;
> > +module_domain_type(test_finitmod_deny_module_request_t)
> > +unconfined_runs_test(test_finitmod_deny_module_request_t)
> > +typeattribute test_finitmod_deny_module_request_t testdomain, kmoduledomain;
> > +
> > +allow test_finitmod_deny_module_request_t self:capability { sys_module };
> > +allow test_finitmod_deny_module_request_t test_file_t:system { module_load };
> > +neverallow test_finitmod_deny_module_request_t kernel_t:system { module_request };
> > +
> > +#
> > +############# Test kernel modules with initmod_module(2) ###################
> > +#
> > +type test_initmod_t;
> > +module_domain_type(test_initmod_t)
> > +unconfined_runs_test(test_initmod_t)
> > +typeattribute test_initmod_t testdomain, kmoduledomain;
> > +
> > +allow test_initmod_t self:capability { sys_module };
> > +allow test_initmod_t self:system { module_load };
> > +allow test_initmod_t kernel_t:system { module_request };
> > +
> > +############### Deny cap sys_module ######################
> > +type test_initmod_deny_sys_module_t;
> > +module_domain_type(test_initmod_deny_sys_module_t)
> > +unconfined_runs_test(test_initmod_deny_sys_module_t)
> > +typeattribute test_initmod_deny_sys_module_t testdomain, kmoduledomain;
> > +
> > +neverallow test_initmod_deny_sys_module_t self:capability { sys_module };
> > +
> > +############### Deny sys module_load ######################
> > +type test_initmod_deny_module_load_t;
> > +module_domain_type(test_initmod_deny_module_load_t)
> > +unconfined_runs_test(test_initmod_deny_module_load_t)
> > +typeattribute test_initmod_deny_module_load_t testdomain, kmoduledomain;
> > +
> > +allow test_initmod_deny_module_load_t self:capability { sys_module };
> > +neverallow test_initmod_deny_module_load_t self:system { module_load };
> > +
> > +############### Deny sys module_request ######################
> > +type test_initmod_deny_module_request_t;
> > +module_domain_type(test_initmod_deny_module_request_t)
> > +unconfined_runs_test(test_initmod_deny_module_request_t)
> > +typeattribute test_initmod_deny_module_request_t testdomain, kmoduledomain;
> > +
> > +allow test_initmod_deny_module_request_t self:capability { sys_module };
> > +allow test_initmod_deny_module_request_t self:system { module_load };
> > +neverallow test_initmod_deny_module_request_t kernel_t:system { module_request };
> > +
> > +#
> > +########### Allow these domains to be entered from sysadm domain ############
> > +#
> > +miscfiles_domain_entry_test_files(kmoduledomain)
> > +userdom_sysadm_entry_spec_domtrans_to(kmoduledomain)
> > diff --git a/tests/Makefile b/tests/Makefile
> > index cca6648..0452887 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -72,6 +72,10 @@ ifeq ($(shell grep -q all_file_perms.*watch $(POLDEV)/include/support/all_perms.
> >  SUBDIRS+=notify
> >  endif
> >
> > +ifeq ($(shell grep -q module_load $(POLDEV)/include/support/all_perms.spt && echo true),true)

When trying this on RHEL-7, I realized there is a missing kernel
version check here (module_load kernel support was introduced in v4.7
[1]):

ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1)
...
endif

It is quite a corner case to have a policy that supports module_load
and a kernel that does not, but there is at least one distro that hits
it, so I think it's worth it to add the explicit check.

Stephen/Paul, is it trivial enough to add when merging the patch or
should Richard repost it?

[1] https://github.com/torvalds/linux/commit/61d612ea731e57dc510472fb746b55cdc017f371

> > +SUBDIRS+=module_load
> > +endif
> > +
> >  ifeq ($(DISTRO),RHEL4)
> >      SUBDIRS:=$(filter-out bounds dyntrace dyntrans inet_socket mmap nnp_nosuid overlay unix_socket, $(SUBDIRS))
> >  endif
> > diff --git a/tests/module_load/.gitignore b/tests/module_load/.gitignore
> > new file mode 100644
> > index 0000000..7fa5772
> > --- /dev/null
> > +++ b/tests/module_load/.gitignore
> > @@ -0,0 +1,11 @@
> > +finit_load
> > +init_load
> > +modules.order
> > +Module.symvers
> > +*.a
> > +*.o
> > +*.ko
> > +*.cmd
> > +*.mod
> > +*.mod.c
> > +.*.cmd
> > diff --git a/tests/module_load/Makefile b/tests/module_load/Makefile
> > new file mode 100644
> > index 0000000..b6eba25
> > --- /dev/null
> > +++ b/tests/module_load/Makefile
> > @@ -0,0 +1,12 @@
> > +obj-m = setest_module_load.o setest_module_request.o
> > +
> > +TARGETS = finit_load init_load
> > +LDLIBS += -lselinux
> > +KDIR = /lib/modules/$(shell uname -r)/build
> > +
> > +all: $(TARGETS)
> > +       $(MAKE) -C $(KDIR) M=$(PWD)
> > +
> > +clean:
> > +       rm -f $(TARGETS)
> > +       rm -f *.a *.o *.ko *.cmd *.mod *.mod.c .*.cmd Module.symvers modules.order
> > diff --git a/tests/module_load/finit_load.c b/tests/module_load/finit_load.c
> > new file mode 100644
> > index 0000000..1c05d7b
> > --- /dev/null
> > +++ b/tests/module_load/finit_load.c
> > @@ -0,0 +1,94 @@
> > +#define _GNU_SOURCE 1
> > +
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <fcntl.h>
> > +#include <limits.h>
> > +#include <sys/syscall.h>
> > +#include <selinux/selinux.h>
> > +
> > +static void print_usage(char *progfile_name)
> > +{
> > +       fprintf(stderr,
> > +               "usage:  %s [-v] path name\n"
> > +               "Where:\n\t"
> > +               "-v    Print information.\n\t"
> > +               "path  Kernel module build path.\n\t"
> > +               "name  Name of kernel module to load.\n", progfile_name);
> > +       exit(-1);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +       char *context, file_name[PATH_MAX];
> > +       int opt, result, fd, s_errno;
> > +       bool verbose = false;
> > +
> > +       while ((opt = getopt(argc, argv, "v")) != -1) {
> > +               switch (opt) {
> > +               case 'v':
> > +                       verbose = true;
> > +                       break;
> > +               default:
> > +                       print_usage(argv[0]);
> > +               }
> > +       }
> > +
> > +       if (optind >= argc)
> > +               print_usage(argv[0]);
> > +
> > +       result = sprintf(file_name, "%s/%s.ko", argv[optind],
> > +                        argv[optind + 1]);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed sprintf\n");
> > +               exit(-1);
> > +       }
> > +
> > +       fd = open(file_name, O_RDONLY);
> > +       if (!fd) {
> > +               fprintf(stderr, "Failed to open %s: %s\n",
> > +                       file_name, strerror(errno));
> > +               exit(-1);
> > +       }
> > +
> > +       if (verbose) {
> > +               result = getcon(&context);
> > +               if (result < 0) {
> > +                       fprintf(stderr, "Failed to obtain process context\n");
> > +                       close(fd);
> > +                       exit(-1);
> > +               }
> > +
> > +               printf("Process context:\n\t%s\n", context);
> > +               free(context);
> > +       }
> > +
> > +       result = syscall(__NR_finit_module, fd, "", 0);
> > +       s_errno = errno;
> > +       close(fd);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed to load '%s' module: %s\n",
> > +                       file_name, strerror(s_errno));
> > +               /* Denying: sys_module=EPERM, module_load=EACCES */
> > +               exit(s_errno);
> > +       }
> > +
> > +       if (verbose)
> > +               printf("Loaded kernel module:  %s\n", file_name);
> > +
> > +       result = syscall(__NR_delete_module, argv[optind + 1], 0);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed to delete '%s' module: %s\n",
> > +                       argv[optind + 1], strerror(errno));
> > +               exit(-1);
> > +       }
> > +
> > +       if (verbose)
> > +               printf("Deleted kernel module: %s\n", argv[optind + 1]);
> > +
> > +       return 0;
> > +}
> > diff --git a/tests/module_load/init_load.c b/tests/module_load/init_load.c
> > new file mode 100644
> > index 0000000..0422c19
> > --- /dev/null
> > +++ b/tests/module_load/init_load.c
> > @@ -0,0 +1,123 @@
> > +#define _GNU_SOURCE 1
> > +
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <fcntl.h>
> > +#include <limits.h>
> > +#include <sys/stat.h>
> > +#include <sys/syscall.h>
> > +#include <selinux/selinux.h>
> > +
> > +static void print_usage(char *progfile_name)
> > +{
> > +       fprintf(stderr,
> > +               "usage:  %s [-v] path name\n"
> > +               "Where:\n\t"
> > +               "-v    Print information.\n\t"
> > +               "path  Kernel module build path.\n\t"
> > +               "name  Name of kernel module to load.\n", progfile_name);
> > +       exit(-1);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +       char *context, file_name[PATH_MAX];
> > +       int opt, result, fd, s_errno;
> > +       bool verbose = false;
> > +       void *elf_image;
> > +       struct stat st;
> > +
> > +       while ((opt = getopt(argc, argv, "v")) != -1) {
> > +               switch (opt) {
> > +               case 'v':
> > +                       verbose = true;
> > +                       break;
> > +               default:
> > +                       print_usage(argv[0]);
> > +               }
> > +       }
> > +
> > +       if (optind >= argc)
> > +               print_usage(argv[0]);
> > +
> > +       result = sprintf(file_name, "%s/%s.ko", argv[optind],
> > +                        argv[optind + 1]);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed sprintf\n");
> > +               exit(-1);
> > +       }
> > +
> > +       fd = open(file_name, O_RDONLY);
> > +       if (!fd) {
> > +               fprintf(stderr, "Failed to open %s: %s\n",
> > +                       file_name, strerror(errno));
> > +               exit(-1);
> > +       }
> > +
> > +       if (verbose) {
> > +               result = getcon(&context);
> > +               if (result < 0) {
> > +                       fprintf(stderr, "Failed to obtain process context\n");
> > +                       close(fd);
> > +                       exit(-1);
> > +               }
> > +
> > +               printf("Process context:\n\t%s\n", context);
> > +               free(context);
> > +       }
> > +
> > +       result = fstat(fd, &st);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed fstat on %s: %s\n",
> > +                       file_name, strerror(errno));
> > +               close(fd);
> > +               exit(-1);
> > +       }
> > +
> > +       elf_image = malloc(st.st_size);
> > +       if (!elf_image) {
> > +               fprintf(stderr, "Failed malloc on %s: %s\n",
> > +                       file_name, strerror(errno));
> > +               close(fd);
> > +               exit(-1);
> > +       }
> > +
> > +       result = read(fd, elf_image, st.st_size);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed read on %s: %s\n",
> > +                       file_name, strerror(errno));
> > +               close(fd);
> > +               free(elf_image);
> > +               exit(-1);
> > +       }
> > +       close(fd);
> > +
> > +       result = syscall(__NR_init_module, elf_image, st.st_size, "");
> > +       s_errno = errno;
> > +       free(elf_image);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed to load '%s' module: %s\n",
> > +                       file_name, strerror(s_errno));
> > +               /* Denying: sys_module=EPERM, module_load & request=EACCES */
> > +               exit(s_errno);
> > +       }
> > +
> > +       if (verbose)
> > +               printf("Loaded kernel module:  %s\n", file_name);
> > +
> > +       result = syscall(__NR_delete_module, argv[optind + 1], 0);
> > +       if (result < 0) {
> > +               fprintf(stderr, "Failed to delete '%s' module: %s\n",
> > +                       argv[optind + 1], strerror(errno));
> > +               exit(-1);
> > +       }
> > +
> > +       if (verbose)
> > +               printf("Deleted kernel module: %s\n", argv[optind + 1]);
> > +
> > +       return 0;
> > +}
> > diff --git a/tests/module_load/setest_module_load.c b/tests/module_load/setest_module_load.c
> > new file mode 100644
> > index 0000000..0be7a26
> > --- /dev/null
> > +++ b/tests/module_load/setest_module_load.c
> > @@ -0,0 +1,18 @@
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +
> > +static int __init setest_module_load_init(void)
> > +{
> > +       pr_info("INIT - setest_module_load\n");
> > +       return 0;
> > +}
> > +
> > +static void __exit setest_module_load_exit(void)
> > +{
> > +       pr_info("EXIT - setest_module_load\n");
> > +}
> > +
> > +module_init(setest_module_load_init);
> > +module_exit(setest_module_load_exit);
> > +MODULE_LICENSE("GPL");
> > diff --git a/tests/module_load/setest_module_request.c b/tests/module_load/setest_module_request.c
> > new file mode 100644
> > index 0000000..f79d4ef
> > --- /dev/null
> > +++ b/tests/module_load/setest_module_request.c
> > @@ -0,0 +1,22 @@
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +
> > +static int __init setest_module_request_init(void)
> > +{
> > +       int result;
> > +
> > +       pr_info("INIT - setest_module_request\n");
> > +       result = request_module_nowait("dummy-module");
> > +       pr_info("request_module() returned: %d\n", result);
> > +       return result;
> > +}
> > +
> > +static void __exit setest_module_request_exit(void)
> > +{
> > +       pr_info("EXIT - setest_module_request\n");
> > +}
> > +
> > +module_init(setest_module_request_init);
> > +module_exit(setest_module_request_exit);
> > +MODULE_LICENSE("GPL");
> > diff --git a/tests/module_load/test b/tests/module_load/test
> > new file mode 100755
> > index 0000000..c3242fc
> > --- /dev/null
> > +++ b/tests/module_load/test
> > @@ -0,0 +1,62 @@
> > +#!/usr/bin/perl
> > +use Test::More;
> > +
> > +BEGIN {
> > +    $basedir = $0;
> > +    $basedir =~ s|(.*)/[^/]*|$1|;
> > +
> > +    # allow info to be shown during tests
> > +    $v = $ARGV[0];
> > +    if ($v) {
> > +        if ( $v ne "-v" ) {
> > +            plan skip_all => "Invalid option (use -v)";
> > +        }
> > +    }
> > +    else {
> > +        $v = " ";
> > +    }
> > +
> > +    plan tests => 8;
> > +}
> > +
> > +print "Test finit_module(2)\n";
> > +$result = system
> > +"runcon -t test_finitmod_t $basedir/finit_load $v $basedir setest_module_request";
> > +ok( $result eq 0 );
> > +
> > +# Deny capability { sys_module } - EPERM
> > +$result = system
> > +"runcon -t test_finitmod_deny_sys_module_t $basedir/finit_load $v $basedir setest_module_load 2>&1";
> > +ok( $result >> 8 eq 1 );
> > +
> > +# Deny system { module_load } - EACCES
> > +$result = system
> > +"runcon -t test_finitmod_deny_module_load_t $basedir/finit_load $v $basedir setest_module_load 2>&1";
> > +ok( $result >> 8 eq 13 );
> > +
> > +# Deny system { module_request } - EACCES
> > +$result = system
> > +"runcon -t test_finitmod_deny_module_request_t $basedir/finit_load $v $basedir setest_module_request 2>&1";
> > +ok( $result >> 8 eq 13 );
> > +
> > +print "Test init_module(2)\n";
> > +$result = system
> > +"runcon -t test_initmod_t $basedir/init_load $v $basedir setest_module_request";
> > +ok( $result eq 0 );
> > +
> > +# Deny capability { sys_module } - EPERM
> > +$result = system
> > +"runcon -t test_initmod_deny_sys_module_t $basedir/init_load $v $basedir setest_module_load 2>&1";
> > +ok( $result >> 8 eq 1 );
> > +
> > +# Deny system { module_load } - EACCES
> > +$result = system
> > +"runcon -t test_initmod_deny_module_load_t $basedir/init_load $v $basedir setest_module_load 2>&1";
> > +ok( $result >> 8 eq 13 );
> > +
> > +# Deny system { module_request } - EACCES
> > +$result = system
> > +"runcon -t test_initmod_deny_module_request_t $basedir/init_load $v $basedir setest_module_request 2>&1";
> > +ok( $result >> 8 eq 13 );
> > +
> > +exit;
> > --
> > 2.23.0
> >
>
>
> --
> Ondrej Mosnacek <omosnace at redhat dot com>
> Software Engineer, Security Technologies
> Red Hat, Inc.
Stephen Smalley Nov. 22, 2019, 1:04 p.m. UTC | #3
On 11/22/19 5:40 AM, Ondrej Mosnacek wrote:
> When trying this on RHEL-7, I realized there is a missing kernel
> version check here (module_load kernel support was introduced in v4.7
> [1]):
> 
> ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1)
> ...
> endif
> 
> It is quite a corner case to have a policy that supports module_load
> and a kernel that does not, but there is at least one distro that hits
> it, so I think it's worth it to add the explicit check.
> 
> Stephen/Paul, is it trivial enough to add when merging the patch or
> should Richard repost it?
> 
> [1] https://github.com/torvalds/linux/commit/61d612ea731e57dc510472fb746b55cdc017f371

Alternatively, you could exclude it on RHEL7 by adding it to the 
filter-out line under ifeq ($(DISTRO),RHEL7).  Regardless, it is 
entirely up to you as to whether you want to fix it up on merge yourself 
or request a re-spin.
Paul Moore Nov. 22, 2019, 1:39 p.m. UTC | #4
On Fri, Nov 22, 2019 at 8:04 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 11/22/19 5:40 AM, Ondrej Mosnacek wrote:
> > When trying this on RHEL-7, I realized there is a missing kernel
> > version check here (module_load kernel support was introduced in v4.7
> > [1]):
> >
> > ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1)
> > ...
> > endif
> >
> > It is quite a corner case to have a policy that supports module_load
> > and a kernel that does not, but there is at least one distro that hits
> > it, so I think it's worth it to add the explicit check.
> >
> > Stephen/Paul, is it trivial enough to add when merging the patch or
> > should Richard repost it?
> >
> > [1] https://github.com/torvalds/linux/commit/61d612ea731e57dc510472fb746b55cdc017f371
>
> Alternatively, you could exclude it on RHEL7 by adding it to the
> filter-out line under ifeq ($(DISTRO),RHEL7).  Regardless, it is
> entirely up to you as to whether you want to fix it up on merge yourself
> or request a re-spin.

Yes, each project and maintainer handle this differently.  I
personally try to have the original submitter make the change(s)
whenever possible (assuming they are not merge related); I do this for
a few reasons: 1) it is their name on the code, and I don't want to
cause them problems by others mistaking my changes for the submitters
changes (aside, this is why I try to mark my edits in the commit
metadata) 2) changes made during the merge don't go through the normal
mailing list review 3) the mailing list feedback loop is a Very Good
Thing in my opinion and we should do what we can to keep it going.

That said, there are cases where the change is mindlessly trivial
(e.g. misspellings, nonconformant whitespace, etc.) and I don't have a
problem making that edit.  I'll also consider making larger changes if
I know the original submitter to be unreliable and we want/need to get
the commit into the tree soon.
Richard Haines Nov. 22, 2019, 4:18 p.m. UTC | #5
On Fri, 2019-11-22 at 08:39 -0500, Paul Moore wrote:
> On Fri, Nov 22, 2019 at 8:04 AM Stephen Smalley <sds@tycho.nsa.gov>
> wrote:
> > On 11/22/19 5:40 AM, Ondrej Mosnacek wrote:
> > > When trying this on RHEL-7, I realized there is a missing kernel
> > > version check here (module_load kernel support was introduced in
> > > v4.7
> > > [1]):
> > > 
> > > ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1)
> > > ...
> > > endif
> > > 
> > > It is quite a corner case to have a policy that supports
> > > module_load
> > > and a kernel that does not, but there is at least one distro that
> > > hits
> > > it, so I think it's worth it to add the explicit check.
> > > 
> > > Stephen/Paul, is it trivial enough to add when merging the patch
> > > or
> > > should Richard repost it?
> > > 
> > > [1] 
> > > https://github.com/torvalds/linux/commit/61d612ea731e57dc510472fb746b55cdc017f371
> > 
> > Alternatively, you could exclude it on RHEL7 by adding it to the
> > filter-out line under ifeq ($(DISTRO),RHEL7).  Regardless, it is
> > entirely up to you as to whether you want to fix it up on merge
> > yourself
> > or request a re-spin.
> 
> Yes, each project and maintainer handle this differently.  I
> personally try to have the original submitter make the change(s)
> whenever possible (assuming they are not merge related); I do this
> for
> a few reasons: 1) it is their name on the code, and I don't want to
> cause them problems by others mistaking my changes for the submitters
> changes (aside, this is why I try to mark my edits in the commit
> metadata) 2) changes made during the merge don't go through the
> normal
> mailing list review 3) the mailing list feedback loop is a Very Good
> Thing in my opinion and we should do what we can to keep it going.
> 
> That said, there are cases where the change is mindlessly trivial
> (e.g. misspellings, nonconformant whitespace, etc.) and I don't have
> a
> problem making that edit.  I'll also consider making larger changes
> if
> I know the original submitter to be unreliable and we want/need to
> get
> the commit into the tree soon.

I'll submit a new patch with:
 ifneq ($(shell ./kvercmp $$(uname -r) 4.7),-1)

Richard
>

Patch
diff mbox series

diff --git a/policy/Makefile b/policy/Makefile
index ad94c43..25dfb69 100644
--- a/policy/Makefile
+++ b/policy/Makefile
@@ -94,6 +94,10 @@  ifeq ($(shell grep -q key_socket $(POLDEV)/include/support/all_perms.spt && echo
 TARGETS += test_key_socket.te
 endif
 
+ifeq ($(shell grep -q module_load $(POLDEV)/include/support/all_perms.spt && echo true),true)
+TARGETS+=test_module_load.te
+endif
+
 ifeq (x$(DISTRO),$(filter x$(DISTRO),xRHEL4 xRHEL5 xRHEL6))
 TARGETS:=$(filter-out test_overlayfs.te test_mqueue.te test_ibpkey.te, $(TARGETS))
 endif
diff --git a/policy/test_module_load.te b/policy/test_module_load.te
new file mode 100644
index 0000000..5496d86
--- /dev/null
+++ b/policy/test_module_load.te
@@ -0,0 +1,108 @@ 
+############# Test kernel modules ###################
+#
+attribute kmoduledomain;
+
+#
+############################## Define Macro ################################
+#
+# Replace domain_type() macro as it hides some relevant denials in audit.log
+#
+gen_require(`
+	type setrans_var_run_t, syslogd_t;
+')
+
+define(`module_domain_type',`
+	allow $1 proc_t:dir { search };
+	allow $1 proc_t:lnk_file { read };
+	allow $1 self:dir { search };
+	allow $1 self:file { open read write };
+	dontaudit init_t syslogd_t:fd use;
+	dontaudit $1 security_t:filesystem getattr;
+	dontaudit $1 self:file getattr;
+	dontaudit $1 setrans_var_run_t:dir search;
+	dontaudit unconfined_t $1:process { noatsecure rlimitinh siginh };
+')
+
+#
+############# Test kernel modules with finitmod_module(2) ###################
+#
+type test_finitmod_t;
+module_domain_type(test_finitmod_t)
+unconfined_runs_test(test_finitmod_t)
+typeattribute test_finitmod_t testdomain, kmoduledomain;
+
+allow test_finitmod_t self:capability { sys_module };
+allow test_finitmod_t test_file_t:system { module_load };
+allow test_finitmod_t kernel_t:system { module_request };
+
+############### Deny cap sys_module ######################
+type test_finitmod_deny_sys_module_t;
+module_domain_type(test_finitmod_deny_sys_module_t)
+unconfined_runs_test(test_finitmod_deny_sys_module_t)
+typeattribute test_finitmod_deny_sys_module_t testdomain, kmoduledomain;
+
+neverallow test_finitmod_deny_sys_module_t self:capability { sys_module };
+
+############### Deny sys module_load ######################
+type test_finitmod_deny_module_load_t;
+module_domain_type(test_finitmod_deny_module_load_t)
+unconfined_runs_test(test_finitmod_deny_module_load_t)
+typeattribute test_finitmod_deny_module_load_t testdomain, kmoduledomain;
+
+allow test_finitmod_deny_module_load_t self:capability { sys_module };
+neverallow test_finitmod_deny_module_load_t test_file_t:system { module_load };
+
+############### Deny sys module_request ######################
+type test_finitmod_deny_module_request_t;
+module_domain_type(test_finitmod_deny_module_request_t)
+unconfined_runs_test(test_finitmod_deny_module_request_t)
+typeattribute test_finitmod_deny_module_request_t testdomain, kmoduledomain;
+
+allow test_finitmod_deny_module_request_t self:capability { sys_module };
+allow test_finitmod_deny_module_request_t test_file_t:system { module_load };
+neverallow test_finitmod_deny_module_request_t kernel_t:system { module_request };
+
+#
+############# Test kernel modules with initmod_module(2) ###################
+#
+type test_initmod_t;
+module_domain_type(test_initmod_t)
+unconfined_runs_test(test_initmod_t)
+typeattribute test_initmod_t testdomain, kmoduledomain;
+
+allow test_initmod_t self:capability { sys_module };
+allow test_initmod_t self:system { module_load };
+allow test_initmod_t kernel_t:system { module_request };
+
+############### Deny cap sys_module ######################
+type test_initmod_deny_sys_module_t;
+module_domain_type(test_initmod_deny_sys_module_t)
+unconfined_runs_test(test_initmod_deny_sys_module_t)
+typeattribute test_initmod_deny_sys_module_t testdomain, kmoduledomain;
+
+neverallow test_initmod_deny_sys_module_t self:capability { sys_module };
+
+############### Deny sys module_load ######################
+type test_initmod_deny_module_load_t;
+module_domain_type(test_initmod_deny_module_load_t)
+unconfined_runs_test(test_initmod_deny_module_load_t)
+typeattribute test_initmod_deny_module_load_t testdomain, kmoduledomain;
+
+allow test_initmod_deny_module_load_t self:capability { sys_module };
+neverallow test_initmod_deny_module_load_t self:system { module_load };
+
+############### Deny sys module_request ######################
+type test_initmod_deny_module_request_t;
+module_domain_type(test_initmod_deny_module_request_t)
+unconfined_runs_test(test_initmod_deny_module_request_t)
+typeattribute test_initmod_deny_module_request_t testdomain, kmoduledomain;
+
+allow test_initmod_deny_module_request_t self:capability { sys_module };
+allow test_initmod_deny_module_request_t self:system { module_load };
+neverallow test_initmod_deny_module_request_t kernel_t:system { module_request };
+
+#
+########### Allow these domains to be entered from sysadm domain ############
+#
+miscfiles_domain_entry_test_files(kmoduledomain)
+userdom_sysadm_entry_spec_domtrans_to(kmoduledomain)
diff --git a/tests/Makefile b/tests/Makefile
index cca6648..0452887 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -72,6 +72,10 @@  ifeq ($(shell grep -q all_file_perms.*watch $(POLDEV)/include/support/all_perms.
 SUBDIRS+=notify
 endif
 
+ifeq ($(shell grep -q module_load $(POLDEV)/include/support/all_perms.spt && echo true),true)
+SUBDIRS+=module_load
+endif
+
 ifeq ($(DISTRO),RHEL4)
     SUBDIRS:=$(filter-out bounds dyntrace dyntrans inet_socket mmap nnp_nosuid overlay unix_socket, $(SUBDIRS))
 endif
diff --git a/tests/module_load/.gitignore b/tests/module_load/.gitignore
new file mode 100644
index 0000000..7fa5772
--- /dev/null
+++ b/tests/module_load/.gitignore
@@ -0,0 +1,11 @@ 
+finit_load
+init_load
+modules.order
+Module.symvers
+*.a
+*.o
+*.ko
+*.cmd
+*.mod
+*.mod.c
+.*.cmd
diff --git a/tests/module_load/Makefile b/tests/module_load/Makefile
new file mode 100644
index 0000000..b6eba25
--- /dev/null
+++ b/tests/module_load/Makefile
@@ -0,0 +1,12 @@ 
+obj-m = setest_module_load.o setest_module_request.o
+
+TARGETS = finit_load init_load
+LDLIBS += -lselinux
+KDIR = /lib/modules/$(shell uname -r)/build
+
+all: $(TARGETS)
+	$(MAKE) -C $(KDIR) M=$(PWD)
+
+clean:
+	rm -f $(TARGETS)
+	rm -f *.a *.o *.ko *.cmd *.mod *.mod.c .*.cmd Module.symvers modules.order
diff --git a/tests/module_load/finit_load.c b/tests/module_load/finit_load.c
new file mode 100644
index 0000000..1c05d7b
--- /dev/null
+++ b/tests/module_load/finit_load.c
@@ -0,0 +1,94 @@ 
+#define _GNU_SOURCE 1
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sys/syscall.h>
+#include <selinux/selinux.h>
+
+static void print_usage(char *progfile_name)
+{
+	fprintf(stderr,
+		"usage:  %s [-v] path name\n"
+		"Where:\n\t"
+		"-v    Print information.\n\t"
+		"path  Kernel module build path.\n\t"
+		"name  Name of kernel module to load.\n", progfile_name);
+	exit(-1);
+}
+
+int main(int argc, char *argv[])
+{
+	char *context, file_name[PATH_MAX];
+	int opt, result, fd, s_errno;
+	bool verbose = false;
+
+	while ((opt = getopt(argc, argv, "v")) != -1) {
+		switch (opt) {
+		case 'v':
+			verbose = true;
+			break;
+		default:
+			print_usage(argv[0]);
+		}
+	}
+
+	if (optind >= argc)
+		print_usage(argv[0]);
+
+	result = sprintf(file_name, "%s/%s.ko", argv[optind],
+			 argv[optind + 1]);
+	if (result < 0) {
+		fprintf(stderr, "Failed sprintf\n");
+		exit(-1);
+	}
+
+	fd = open(file_name, O_RDONLY);
+	if (!fd) {
+		fprintf(stderr, "Failed to open %s: %s\n",
+			file_name, strerror(errno));
+		exit(-1);
+	}
+
+	if (verbose) {
+		result = getcon(&context);
+		if (result < 0) {
+			fprintf(stderr, "Failed to obtain process context\n");
+			close(fd);
+			exit(-1);
+		}
+
+		printf("Process context:\n\t%s\n", context);
+		free(context);
+	}
+
+	result = syscall(__NR_finit_module, fd, "", 0);
+	s_errno = errno;
+	close(fd);
+	if (result < 0) {
+		fprintf(stderr, "Failed to load '%s' module: %s\n",
+			file_name, strerror(s_errno));
+		/* Denying: sys_module=EPERM, module_load=EACCES */
+		exit(s_errno);
+	}
+
+	if (verbose)
+		printf("Loaded kernel module:  %s\n", file_name);
+
+	result = syscall(__NR_delete_module, argv[optind + 1], 0);
+	if (result < 0) {
+		fprintf(stderr, "Failed to delete '%s' module: %s\n",
+			argv[optind + 1], strerror(errno));
+		exit(-1);
+	}
+
+	if (verbose)
+		printf("Deleted kernel module: %s\n", argv[optind + 1]);
+
+	return 0;
+}
diff --git a/tests/module_load/init_load.c b/tests/module_load/init_load.c
new file mode 100644
index 0000000..0422c19
--- /dev/null
+++ b/tests/module_load/init_load.c
@@ -0,0 +1,123 @@ 
+#define _GNU_SOURCE 1
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <selinux/selinux.h>
+
+static void print_usage(char *progfile_name)
+{
+	fprintf(stderr,
+		"usage:  %s [-v] path name\n"
+		"Where:\n\t"
+		"-v    Print information.\n\t"
+		"path  Kernel module build path.\n\t"
+		"name  Name of kernel module to load.\n", progfile_name);
+	exit(-1);
+}
+
+int main(int argc, char *argv[])
+{
+	char *context, file_name[PATH_MAX];
+	int opt, result, fd, s_errno;
+	bool verbose = false;
+	void *elf_image;
+	struct stat st;
+
+	while ((opt = getopt(argc, argv, "v")) != -1) {
+		switch (opt) {
+		case 'v':
+			verbose = true;
+			break;
+		default:
+			print_usage(argv[0]);
+		}
+	}
+
+	if (optind >= argc)
+		print_usage(argv[0]);
+
+	result = sprintf(file_name, "%s/%s.ko", argv[optind],
+			 argv[optind + 1]);
+	if (result < 0) {
+		fprintf(stderr, "Failed sprintf\n");
+		exit(-1);
+	}
+
+	fd = open(file_name, O_RDONLY);
+	if (!fd) {
+		fprintf(stderr, "Failed to open %s: %s\n",
+			file_name, strerror(errno));
+		exit(-1);
+	}
+
+	if (verbose) {
+		result = getcon(&context);
+		if (result < 0) {
+			fprintf(stderr, "Failed to obtain process context\n");
+			close(fd);
+			exit(-1);
+		}
+
+		printf("Process context:\n\t%s\n", context);
+		free(context);
+	}
+
+	result = fstat(fd, &st);
+	if (result < 0) {
+		fprintf(stderr, "Failed fstat on %s: %s\n",
+			file_name, strerror(errno));
+		close(fd);
+		exit(-1);
+	}
+
+	elf_image = malloc(st.st_size);
+	if (!elf_image) {
+		fprintf(stderr, "Failed malloc on %s: %s\n",
+			file_name, strerror(errno));
+		close(fd);
+		exit(-1);
+	}
+
+	result = read(fd, elf_image, st.st_size);
+	if (result < 0) {
+		fprintf(stderr, "Failed read on %s: %s\n",
+			file_name, strerror(errno));
+		close(fd);
+		free(elf_image);
+		exit(-1);
+	}
+	close(fd);
+
+	result = syscall(__NR_init_module, elf_image, st.st_size, "");
+	s_errno = errno;
+	free(elf_image);
+	if (result < 0) {
+		fprintf(stderr, "Failed to load '%s' module: %s\n",
+			file_name, strerror(s_errno));
+		/* Denying: sys_module=EPERM, module_load & request=EACCES */
+		exit(s_errno);
+	}
+
+	if (verbose)
+		printf("Loaded kernel module:  %s\n", file_name);
+
+	result = syscall(__NR_delete_module, argv[optind + 1], 0);
+	if (result < 0) {
+		fprintf(stderr, "Failed to delete '%s' module: %s\n",
+			argv[optind + 1], strerror(errno));
+		exit(-1);
+	}
+
+	if (verbose)
+		printf("Deleted kernel module: %s\n", argv[optind + 1]);
+
+	return 0;
+}
diff --git a/tests/module_load/setest_module_load.c b/tests/module_load/setest_module_load.c
new file mode 100644
index 0000000..0be7a26
--- /dev/null
+++ b/tests/module_load/setest_module_load.c
@@ -0,0 +1,18 @@ 
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+static int __init setest_module_load_init(void)
+{
+	pr_info("INIT - setest_module_load\n");
+	return 0;
+}
+
+static void __exit setest_module_load_exit(void)
+{
+	pr_info("EXIT - setest_module_load\n");
+}
+
+module_init(setest_module_load_init);
+module_exit(setest_module_load_exit);
+MODULE_LICENSE("GPL");
diff --git a/tests/module_load/setest_module_request.c b/tests/module_load/setest_module_request.c
new file mode 100644
index 0000000..f79d4ef
--- /dev/null
+++ b/tests/module_load/setest_module_request.c
@@ -0,0 +1,22 @@ 
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+static int __init setest_module_request_init(void)
+{
+	int result;
+
+	pr_info("INIT - setest_module_request\n");
+	result = request_module_nowait("dummy-module");
+	pr_info("request_module() returned: %d\n", result);
+	return result;
+}
+
+static void __exit setest_module_request_exit(void)
+{
+	pr_info("EXIT - setest_module_request\n");
+}
+
+module_init(setest_module_request_init);
+module_exit(setest_module_request_exit);
+MODULE_LICENSE("GPL");
diff --git a/tests/module_load/test b/tests/module_load/test
new file mode 100755
index 0000000..c3242fc
--- /dev/null
+++ b/tests/module_load/test
@@ -0,0 +1,62 @@ 
+#!/usr/bin/perl
+use Test::More;
+
+BEGIN {
+    $basedir = $0;
+    $basedir =~ s|(.*)/[^/]*|$1|;
+
+    # allow info to be shown during tests
+    $v = $ARGV[0];
+    if ($v) {
+        if ( $v ne "-v" ) {
+            plan skip_all => "Invalid option (use -v)";
+        }
+    }
+    else {
+        $v = " ";
+    }
+
+    plan tests => 8;
+}
+
+print "Test finit_module(2)\n";
+$result = system
+"runcon -t test_finitmod_t $basedir/finit_load $v $basedir setest_module_request";
+ok( $result eq 0 );
+
+# Deny capability { sys_module } - EPERM
+$result = system
+"runcon -t test_finitmod_deny_sys_module_t $basedir/finit_load $v $basedir setest_module_load 2>&1";
+ok( $result >> 8 eq 1 );
+
+# Deny system { module_load } - EACCES
+$result = system
+"runcon -t test_finitmod_deny_module_load_t $basedir/finit_load $v $basedir setest_module_load 2>&1";
+ok( $result >> 8 eq 13 );
+
+# Deny system { module_request } - EACCES
+$result = system
+"runcon -t test_finitmod_deny_module_request_t $basedir/finit_load $v $basedir setest_module_request 2>&1";
+ok( $result >> 8 eq 13 );
+
+print "Test init_module(2)\n";
+$result = system
+"runcon -t test_initmod_t $basedir/init_load $v $basedir setest_module_request";
+ok( $result eq 0 );
+
+# Deny capability { sys_module } - EPERM
+$result = system
+"runcon -t test_initmod_deny_sys_module_t $basedir/init_load $v $basedir setest_module_load 2>&1";
+ok( $result >> 8 eq 1 );
+
+# Deny system { module_load } - EACCES
+$result = system
+"runcon -t test_initmod_deny_module_load_t $basedir/init_load $v $basedir setest_module_load 2>&1";
+ok( $result >> 8 eq 13 );
+
+# Deny system { module_request } - EACCES
+$result = system
+"runcon -t test_initmod_deny_module_request_t $basedir/init_load $v $basedir setest_module_request 2>&1";
+ok( $result >> 8 eq 13 );
+
+exit;