[refpolicy] [PATCH v4] Add module_load permission to can_load_kernmodule

[guido@trentalancia.net (Guido Trentalancia)]

Hello Christopher,

thanks for your feedback on this patch !

On Mon, 15/08/2016 at 16.20 -0400, Chris PeBenito wrote:
> On 08/15/16 18:22, Guido Trentalancia wrote:
> > The "module_load" permission has been recently added to the
> > "system"
> > class (kernel 4.7).
> > 
> > The following patch updates the Reference Policy so that the new
> > permission is allowed when a kernel module should be loaded.
> > 
> > A couple of unneeded permissions are removed from the kernel
> > module loading section.
> > 
> > Signed-off-by: Guido Trentalancia <guido@trentalancia.net>
> > ---
> > ?policy/modules/kernel/files.te??|???11 +++++++++++
> > ?policy/modules/kernel/kernel.te |????5 -----
> > ?2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > --- refpolicy-git-06082016-orig/policy/modules/kernel/files.te	
> > 2016-08-06 21:26:43.284774157 +0200
> > +++ refpolicy-git-06082016/policy/modules/kernel/files.te	20
> > 16-08-14 22:35:30.602463332 +0200
> > @@ -208,6 +208,17 @@ fs_associate_tmpfs(tmpfsfile)
> > 
> > ?########################################
> > ?#
> > +# Kernel module loading policy
> > +#
> > +
> > +neverallow ~can_load_kernmodule modules_object_t:system
> > module_load;
> > +
> > +if( ! secure_mode_insmod ) {
> > +	allow can_load_kernmodule modules_object_t:system
> > module_load;
> > +}
> 
> Now we have the problem where can_load_kernmodule isn't owned by
> this?
> module.??You'll have to create the first neverallow interface I
> think.

I am sorry but I do not understand what you mean.

It compiles fine. What change do you propose exactly ?

> > +########################################
> > +#
> > ?# Unconfined access to this module
> > ?#
> > 
> > --- refpolicy-git-06082016-orig/policy/modules/kernel/kernel.te	
> > 2016-08-09 16:09:48.811753763 +0200
> > +++ refpolicy-git-06082016/policy/modules/kernel/kernel.te	2
> > 016-08-16 00:09:58.411688357 +0200
> > @@ -428,11 +428,6 @@ optional_policy(`
> > 
> > ?if( ! secure_mode_insmod ) {
> > ?	allow can_load_kernmodule self:capability sys_module;
> > -
> > -	# load_module() calls stop_machine() which
> > -	# calls sched_setscheduler()
> > -	allow can_load_kernmodule self:capability sys_nice;
> > -	kernel_setsched(can_load_kernmodule)
> > ?}
> > 
> > ?########################################
> 
> I also agree with Dominick on this.??These are relatively
> uninteresting?
> permissions compared to the sys_module, so until we have some?
> explanation, I'd prefer to keep it.??I tried looked at Red Hat
> bugzilla,?
> as this came from Dan Walsh, but it's all the way back from 2011, so?
> it's not coming up easily for me.

That's fine, I'll recreate the patch with the sys_nice permission back
in, although I still think it is much better to avoid granting that.

I will add a short note so that hopefully we'll be able to get an
explanation one day...

Best regards,

Guido