Re: [PATCH testsuite v5] policy: use the kernel_request_load_module() interface

[Stephen Smalley <sds@xxxxxxxxxxxxx>]

On 11/27/19 7:33 AM, Ondrej Mosnacek wrote:
> ...instead of open-coding the rules. Also define a fallback to allow the
> policy to build even if the interface is not defined.
>
> Fixes: f5e5a0b8d005 ("selinux-testsuite: Add key_socket tests")
> Cc: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
> Suggested-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> ---
>
> Change in v5: call the macro only once for the whole domain
> Change in v4: fix copy-paste mistakes spotted by Richard
> Change in v3: use different approach as suggested by Stephen
> Change in v2: update also tests/Makefile for consistency
>
>   policy/test_key_socket.te | 8 +++-----
>   policy/test_policy.if     | 6 ++++++
>   2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/policy/test_key_socket.te b/policy/test_key_socket.te
> index cde426b..64d2cee 100644
> --- a/policy/test_key_socket.te
> +++ b/policy/test_key_socket.te
> @@ -12,8 +12,6 @@ typeattribute test_key_sock_t keysockdomain;
>   # key_socket rules:
>   allow test_key_sock_t self:capability { net_admin };
>   allow test_key_sock_t self:key_socket { create write read setopt };
> -# For CONFIG_NET_KEY=m
> -allow test_key_sock_t kernel_t:system { module_request };
>   
>   ################## Deny capability { net_admin } ##########################
>   #
> @@ -29,7 +27,6 @@ typeattribute test_key_sock_no_net_admin_t testdomain;
>   typeattribute test_key_sock_no_net_admin_t keysockdomain;
>   
>   allow test_key_sock_no_net_admin_t self:key_socket { create write read setopt };
> -allow test_key_sock_no_net_admin_t kernel_t:system { module_request };
>   
>   ####################### Deny key_socket { create } ##########################
>   type test_key_sock_no_create_t;
> @@ -50,7 +47,6 @@ typeattribute test_key_sock_no_write_t keysockdomain;
>   
>   allow test_key_sock_no_write_t self:capability { net_admin };
>   allow test_key_sock_no_write_t self:key_socket { create read setopt };
> -allow test_key_sock_no_write_t kernel_t:system { module_request };
>   
>   ####################### Deny key_socket { read } ##########################
>   type test_key_sock_no_read_t;
> @@ -61,10 +57,12 @@ typeattribute test_key_sock_no_read_t keysockdomain;
>   
>   allow test_key_sock_no_read_t self:capability { net_admin };
>   allow test_key_sock_no_read_t self:key_socket { create write setopt };
> -allow test_key_sock_no_read_t kernel_t:system { module_request };
>   
>   #
>   ########### Allow these domains to be entered from sysadm domain ############
>   #
>   miscfiles_domain_entry_test_files(keysockdomain)
>   userdom_sysadm_entry_spec_domtrans_to(keysockdomain)
> +
> +# For CONFIG_NET_KEY=m
> +kernel_request_load_module(keysockdomain)
> diff --git a/policy/test_policy.if b/policy/test_policy.if
> index e1175e8..3f163cb 100644
> --- a/policy/test_policy.if
> +++ b/policy/test_policy.if
> @@ -82,3 +82,9 @@ interface(`userdom_search_admin_dir', `
>       userdom_search_user_home_content($1)
>   ')
>   ')
> +
> +# If the macro isn't defined, then most probably module_request permission

Sorry, I should have caught this earlier.  m4 does not do the right 
thing with embedded quotes in comments, so you can get weird effects 
after macro expansion.  I'd expand it to "is not" to be safe.

> +# is just not supported (and relevant operations should be just allowed).
> +ifdef(`kernel_request_load_module', `', ` dnl
> +interface(`kernel_request_load_module', `')
> +')