On Tue, 2019-11-26 at 16:02 +0100, Ondrej Mosnacek wrote:
> On Tue, Nov 26, 2019 at 3:54 PM Stephen Smalley <sds@xxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> > > > Suggested-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
> > > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > > > ---
> > > >
> > > > 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', `')
> > > > +')
>
>
