Re: [PATCH] selinux: restrict kernel module loading

[William Roberts <bill.c.roberts@xxxxxxxxx>]

On Sat, Apr 2, 2016 at 8:31 AM, Paul Moore <paul@xxxxxxxxxxxxxx> wrote:

On Fri, Apr 1, 2016 at 6:40 PM, Jeff Vander Stoep <jeffv@xxxxxxxxxx> wrote:
> Utilize existing kernel_read_file hook on kernel module load.
> Add module_load permission to the system class.
>
> Enforces restrictions on kernel module origin when calling the
> finit_module syscall. The hook checks that source type has
> permission module_load for the target type.
> Example for finit_module:
>
> allow foo bar_file:system module_load;
>
> Similarly restrictions are enforced on kernel module loading when
> calling the init_module syscall. The hook checks that source
> type has permission module_load for the kernel target type.
> Example for init_module:
>
> allow foo kernel:system module_load;
>
> Signed-off-by: Jeff Vander Stoep <jeffv@xxxxxxxxxx>
> ---
>  security/selinux/hooks.c            | 52 +++++++++++++++++++++++++++++++++++++
>  security/selinux/include/classmap.h |  2 +-
>  2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3fa3ca5..5bc4875 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3719,6 +3719,57 @@ static int selinux_kernel_module_request(char *kmod_name)
>                             SYSTEM__MODULE_REQUEST, &ad);
>  }
>
> +static int selinux_kernel_module_from_file(struct file *file)
> +{
> +       struct common_audit_data ad;
> +       struct inode_security_struct *isec;
> +       struct file_security_struct *fsec;
> +       struct inode *inode;
> +       u32 sid = current_sid();
> +       int rc;
> +
> +       /* init_module */
> +       if (file == NULL) {
> +               rc = avc_has_perm(sid, SECINITSID_KERNEL, SECCLASS_SYSTEM,
> +                                       SYSTEM__MODULE_LOAD, NULL);
> +               goto out;
> +       }

I don't think the object type is correct here; in this case, the
object is the kernel module being loaded, and with init_module() that
kernel module is coming from the calling process, not the kernel.  It
seems like the following would be more appropriate:

  avc_has_perm(sid, sid, SECCLASS_SYSTEM, SYSTEM__MODULE_LOAD, NULL);

Paul your saying you prefer this:

allow foo self:system module_load;

to this:

allow foo kernel:system module_load;

 

?

IMHO, the target would be the kernel since its what is being affected by a module sourced from foo.

Bill

--
paul moore
www.paul-moore.com

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.

--

Respectfully,

William C Roberts

_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.