| Message ID | 20191126120623.736870-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [testsuite,v3] policy: use the kernel_request_load_module() interface | expand |
On 11/26/19 7:06 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> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > > 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, 10 insertions(+), 4 deletions(-) > > diff --git a/policy/test_key_socket.te b/policy/test_key_socket.te > index cde426b..f1c1606 100644 > --- a/policy/test_key_socket.te > +++ b/policy/test_key_socket.te > @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; > 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 }; > +kernel_request_load_module(test_key_sock_t) > > ################## Deny capability { net_admin } ########################## > # > @@ -29,7 +29,7 @@ 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 }; > +kernel_request_load_module(test_key_sock_t) > > ####################### Deny key_socket { create } ########################## > type test_key_sock_no_create_t; > @@ -50,7 +50,7 @@ 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 }; > +kernel_request_load_module(test_key_sock_t) > > ####################### Deny key_socket { read } ########################## > type test_key_sock_no_read_t; > @@ -61,7 +61,7 @@ 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 }; > +kernel_request_load_module(test_key_sock_t) > > # > ########### Allow these domains to be entered from sysadm domain ############ > 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 Tue, 2019-11-26 at 13:06 +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> > --- > > 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, 10 insertions(+), 4 deletions(-) > > diff --git a/policy/test_key_socket.te b/policy/test_key_socket.te > index cde426b..f1c1606 100644 > --- a/policy/test_key_socket.te > +++ b/policy/test_key_socket.te > @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; > 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 }; > +kernel_request_load_module(test_key_sock_t) > > ################## Deny capability { net_admin } > ########################## > # > @@ -29,7 +29,7 @@ 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 > }; > +kernel_request_load_module(test_key_sock_t) All the new entries have (test_key_sock_t) ?? Anyway if you run the tests in the order they appear in 'test' script, then it just so happens that the module will be loaded for test_key_sock_t as it's first. I added the others just to cover the cases where I sometimes run out of sequence, so you could remove these if required. > > ####################### Deny key_socket { create } > ########################## > type test_key_sock_no_create_t; > @@ -50,7 +50,7 @@ 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 }; > +kernel_request_load_module(test_key_sock_t) > > ####################### Deny key_socket { read } > ########################## > type test_key_sock_no_read_t; > @@ -61,7 +61,7 @@ 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 }; > +kernel_request_load_module(test_key_sock_t) > > # > ########### Allow these domains to be entered from sysadm domain > ############ > 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/26/19 9:51 AM, Richard Haines wrote: > On Tue, 2019-11-26 at 13:06 +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> >> --- >> >> 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, 10 insertions(+), 4 deletions(-) >> >> diff --git a/policy/test_key_socket.te b/policy/test_key_socket.te >> index cde426b..f1c1606 100644 >> --- a/policy/test_key_socket.te >> +++ b/policy/test_key_socket.te >> @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; >> 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 }; >> +kernel_request_load_module(test_key_sock_t) >> >> ################## Deny capability { net_admin } >> ########################## >> # >> @@ -29,7 +29,7 @@ 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 >> }; >> +kernel_request_load_module(test_key_sock_t) > > All the new entries have (test_key_sock_t) ?? > Anyway if you run the tests in the order they appear in 'test' script, > then it just so happens that the module will be loaded for > test_key_sock_t as it's first. I added the others just to cover the > cases where I sometimes run out of sequence, so you could remove these > if required. Good catch! Somehow I missed that. Could probably just have a single kernel_request_load_module(keysockdomain) line? > >> >> ####################### Deny key_socket { create } >> ########################## >> type test_key_sock_no_create_t; >> @@ -50,7 +50,7 @@ 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 }; >> +kernel_request_load_module(test_key_sock_t) >> >> ####################### Deny key_socket { read } >> ########################## >> type test_key_sock_no_read_t; >> @@ -61,7 +61,7 @@ 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 }; >> +kernel_request_load_module(test_key_sock_t) >> >> # >> ########### Allow these domains to be entered from sysadm domain >> ############ >> 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 Tue, 2019-11-26 at 09:54 -0500, Stephen Smalley wrote: > On 11/26/19 9:51 AM, Richard Haines wrote: > > On Tue, 2019-11-26 at 13:06 +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> > > > --- > > > > > > 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, 10 insertions(+), 4 deletions(-) > > > > > > diff --git a/policy/test_key_socket.te > > > b/policy/test_key_socket.te > > > index cde426b..f1c1606 100644 > > > --- a/policy/test_key_socket.te > > > +++ b/policy/test_key_socket.te > > > @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; > > > 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 }; > > > +kernel_request_load_module(test_key_sock_t) > > > > > > ################## Deny capability { net_admin } > > > ########################## > > > # > > > @@ -29,7 +29,7 @@ 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 > > > }; > > > +kernel_request_load_module(test_key_sock_t) > > > > All the new entries have (test_key_sock_t) ?? > > Anyway if you run the tests in the order they appear in 'test' > > script, > > then it just so happens that the module will be loaded for > > test_key_sock_t as it's first. I added the others just to cover the > > cases where I sometimes run out of sequence, so you could remove > > these > > if required. > > Good catch! Somehow I missed that. Could probably just have a > single > kernel_request_load_module(keysockdomain) line? That's fine as it cover all cases. > > > > > > > ####################### Deny key_socket { create } > > > ########################## > > > type test_key_sock_no_create_t; > > > @@ -50,7 +50,7 @@ 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 > > > }; > > > +kernel_request_load_module(test_key_sock_t) > > > > > > ####################### Deny key_socket { read } > > > ########################## > > > type test_key_sock_no_read_t; > > > @@ -61,7 +61,7 @@ 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 > > > }; > > > +kernel_request_load_module(test_key_sock_t) > > > > > > # > > > ########### Allow these domains to be entered from sysadm > > > domain > > > ############ > > > 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 Tue, Nov 26, 2019 at 3:54 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 11/26/19 9:51 AM, Richard Haines wrote: > > On Tue, 2019-11-26 at 13:06 +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> > >> --- > >> > >> 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, 10 insertions(+), 4 deletions(-) > >> > >> diff --git a/policy/test_key_socket.te b/policy/test_key_socket.te > >> index cde426b..f1c1606 100644 > >> --- a/policy/test_key_socket.te > >> +++ b/policy/test_key_socket.te > >> @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; > >> 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 }; > >> +kernel_request_load_module(test_key_sock_t) > >> > >> ################## Deny capability { net_admin } > >> ########################## > >> # > >> @@ -29,7 +29,7 @@ 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 > >> }; > >> +kernel_request_load_module(test_key_sock_t) > > > > All the new entries have (test_key_sock_t) ?? > > Anyway if you run the tests in the order they appear in 'test' script, > > then it just so happens that the module will be loaded for > > test_key_sock_t as it's first. I added the others just to cover the > > cases where I sometimes run out of sequence, so you could remove these > > if required. *facepalm* Yes, sorry, I messed up the copy-paste... > > Good catch! Somehow I missed that. Could probably just have a single > kernel_request_load_module(keysockdomain) line? There is one domain (test_key_sock_no_create_t) that doesn't have the rule, so probably not. > > > > >> > >> ####################### Deny key_socket { create } > >> ########################## > >> type test_key_sock_no_create_t; > >> @@ -50,7 +50,7 @@ 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 }; > >> +kernel_request_load_module(test_key_sock_t) > >> > >> ####################### Deny key_socket { read } > >> ########################## > >> type test_key_sock_no_read_t; > >> @@ -61,7 +61,7 @@ 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 }; > >> +kernel_request_load_module(test_key_sock_t) > >> > >> # > >> ########### Allow these domains to be entered from sysadm domain > >> ############ > >> 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 Tue, 2019-11-26 at 16:02 +0100, Ondrej Mosnacek wrote: > On Tue, Nov 26, 2019 at 3:54 PM Stephen Smalley <sds@tycho.nsa.gov> > wrote: > > On 11/26/19 9:51 AM, Richard Haines wrote: > > > On Tue, 2019-11-26 at 13:06 +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> > > > > --- > > > > > > > > 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, 10 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/policy/test_key_socket.te > > > > b/policy/test_key_socket.te > > > > index cde426b..f1c1606 100644 > > > > --- a/policy/test_key_socket.te > > > > +++ b/policy/test_key_socket.te > > > > @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; > > > > 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 }; > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > ################## Deny capability { net_admin } > > > > ########################## > > > > # > > > > @@ -29,7 +29,7 @@ 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 > > > > }; > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > All the new entries have (test_key_sock_t) ?? > > > Anyway if you run the tests in the order they appear in 'test' > > > script, > > > then it just so happens that the module will be loaded for > > > test_key_sock_t as it's first. I added the others just to cover > > > the > > > cases where I sometimes run out of sequence, so you could remove > > > these > > > if required. > > *facepalm* > > Yes, sorry, I messed up the copy-paste... > > > Good catch! Somehow I missed that. Could probably just have a > > single > > kernel_request_load_module(keysockdomain) line? > > There is one domain (test_key_sock_no_create_t) that doesn't have the > rule, so probably not. That is probably one I didn't test out of sequence. I'm currently writing the TUN/TAP tests and had the same problem as I've left the default Fedora CONFIG_TUN=m. I've added 'kernel_request_load_module(tuntapdomain)' to the test module that allows tests in any order (as I usually run rmmod just as a check). I'll leave it to you guys to decide what is finally best for you. > > > > > ####################### Deny key_socket { create } > > > > ########################## > > > > type test_key_sock_no_create_t; > > > > @@ -50,7 +50,7 @@ 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 }; > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > ####################### Deny key_socket { read } > > > > ########################## > > > > type test_key_sock_no_read_t; > > > > @@ -61,7 +61,7 @@ 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 > > > > }; > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > # > > > > ########### Allow these domains to be entered from sysadm > > > > domain > > > > ############ > > > > 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 Tue, Nov 26, 2019 at 4:59 PM Richard Haines <richard_c_haines@btinternet.com> wrote: > On Tue, 2019-11-26 at 16:02 +0100, Ondrej Mosnacek wrote: > > On Tue, Nov 26, 2019 at 3:54 PM Stephen Smalley <sds@tycho.nsa.gov> > > wrote: > > > On 11/26/19 9:51 AM, Richard Haines wrote: > > > > On Tue, 2019-11-26 at 13:06 +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> > > > > > --- > > > > > > > > > > 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, 10 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/policy/test_key_socket.te > > > > > b/policy/test_key_socket.te > > > > > index cde426b..f1c1606 100644 > > > > > --- a/policy/test_key_socket.te > > > > > +++ b/policy/test_key_socket.te > > > > > @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; > > > > > 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 }; > > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > > > ################## Deny capability { net_admin } > > > > > ########################## > > > > > # > > > > > @@ -29,7 +29,7 @@ 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 > > > > > }; > > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > All the new entries have (test_key_sock_t) ?? > > > > Anyway if you run the tests in the order they appear in 'test' > > > > script, > > > > then it just so happens that the module will be loaded for > > > > test_key_sock_t as it's first. I added the others just to cover > > > > the > > > > cases where I sometimes run out of sequence, so you could remove > > > > these > > > > if required. > > > > *facepalm* > > > > Yes, sorry, I messed up the copy-paste... > > > > > Good catch! Somehow I missed that. Could probably just have a > > > single > > > kernel_request_load_module(keysockdomain) line? > > > > There is one domain (test_key_sock_no_create_t) that doesn't have the > > rule, so probably not. > > That is probably one I didn't test out of sequence. > > I'm currently writing the TUN/TAP tests and had the same problem as > I've left the default Fedora CONFIG_TUN=m. > I've added 'kernel_request_load_module(tuntapdomain)' to the test > module that allows tests in any order (as I usually run rmmod just as a > check). > > I'll leave it to you guys to decide what is finally best for you. Aha, I see, so actually all the types might potentially need the rule (if someone rmmods the module during test, for example), so it would make more sense to allow it to the whole domain as Stephen suggested. I didn't stop to think about it and I assumed that the one domain didn't have the rule intentionally... I think I'll repost the patch once more, calling the macro on the whole keysockdomain. > > > > > > > > ####################### Deny key_socket { create } > > > > > ########################## > > > > > type test_key_sock_no_create_t; > > > > > @@ -50,7 +50,7 @@ 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 }; > > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > > > ####################### Deny key_socket { read } > > > > > ########################## > > > > > type test_key_sock_no_read_t; > > > > > @@ -61,7 +61,7 @@ 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 > > > > > }; > > > > > +kernel_request_load_module(test_key_sock_t) > > > > > > > > > > # > > > > > ########### Allow these domains to be entered from sysadm > > > > > domain > > > > > ############ > > > > > 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', `') > > > > > +') > > > > >
diff --git a/policy/test_key_socket.te b/policy/test_key_socket.te index cde426b..f1c1606 100644 --- a/policy/test_key_socket.te +++ b/policy/test_key_socket.te @@ -13,7 +13,7 @@ typeattribute test_key_sock_t keysockdomain; 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 }; +kernel_request_load_module(test_key_sock_t) ################## Deny capability { net_admin } ########################## # @@ -29,7 +29,7 @@ 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 }; +kernel_request_load_module(test_key_sock_t) ####################### Deny key_socket { create } ########################## type test_key_sock_no_create_t; @@ -50,7 +50,7 @@ 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 }; +kernel_request_load_module(test_key_sock_t) ####################### Deny key_socket { read } ########################## type test_key_sock_no_read_t; @@ -61,7 +61,7 @@ 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 }; +kernel_request_load_module(test_key_sock_t) # ########### Allow these domains to be entered from sysadm domain ############ 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 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, 10 insertions(+), 4 deletions(-)