| Message ID | 20191127123319.814863-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [testsuite,v5] policy: use the kernel_request_load_module() interface | expand |
On Wed, 2019-11-27 at 13:33 +0100, 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@btinternet.com> > Suggested-by: Stephen Smalley <sds@tycho.nsa.gov> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > Thanks, ack from me > 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 > +# is just not supported (and relevant operations should be just > allowed). > +ifdef(`kernel_request_load_module', `', ` dnl > +interface(`kernel_request_load_module', `') > +')
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@btinternet.com> > Suggested-by: Stephen Smalley <sds@tycho.nsa.gov> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > > 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', `') > +') >
On Wed, Nov 27, 2019 at 4:16 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > 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@btinternet.com> > > Suggested-by: Stephen Smalley <sds@tycho.nsa.gov> > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > --- > > > > 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. I see, will do another respin then. > > > +# is just not supported (and relevant operations should be just allowed). > > +ifdef(`kernel_request_load_module', `', ` dnl > > +interface(`kernel_request_load_module', `') > > +') > > > -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
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 +# is just not supported (and relevant operations should be just allowed). +ifdef(`kernel_request_load_module', `', ` dnl +interface(`kernel_request_load_module', `') +')
...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@btinternet.com> Suggested-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- 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(-)