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@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.
>
> 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', `')
> > > +')
