should module_load be checked on a kernel module object?

should module_load be checked on a kernel module object?

[Dominick Grift]

[-- Attachment #1.1: Type: text/plain, Size: 556 bytes --]


I encountered a system module_load event for the first time today.
Howver i am a bit surprised:


avc:  denied  { module_load } for  pid=473 comm="modprobe"
scontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023
tcontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023 tclass=system
permissive=1

Should that permission not have applied to a kernel module object
instead of "self"?
-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 648 bytes --]

Re: should module_load be checked on a kernel module object?

[Jeffrey Vander Stoep]

When loading a kernel module using init_module the module is copied
from memory of the calling process. In that case, the target really is
the calling process. When using finit_module a file is passed to the
kernel and that file is the target object.

See the commit message that added module_load for a more complete
description: https://marc.info/?l=selinux&m=145988689809307&w=2

On Sun, Oct 9, 2016 at 1:10 AM, Dominick Grift <dac.override@gmail.com> wrote:
>
> I encountered a system module_load event for the first time today.
> Howver i am a bit surprised:
>
>
> avc:  denied  { module_load } for  pid=473 comm="modprobe"
> scontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023
> tcontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023 tclass=system
> permissive=1
>
> Should that permission not have applied to a kernel module object
> instead of "self"?
> --
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

Re: should module_load be checked on a kernel module object?

[Dominick Grift]

[-- Attachment #1.1: Type: text/plain, Size: 1596 bytes --]

On 10/10/2016 05:02 PM, Jeffrey Vander Stoep wrote:
> When loading a kernel module using init_module the module is copied
> from memory of the calling process. In that case, the target really is
> the calling process. When using finit_module a file is passed to the
> kernel and that file is the target object.
> 
> See the commit message that added module_load for a more complete
> description: https://marc.info/?l=selinux&m=145988689809307&w=2
> 

Thanks, Sorry about that.

> On Sun, Oct 9, 2016 at 1:10 AM, Dominick Grift <dac.override@gmail.com> wrote:
>>
>> I encountered a system module_load event for the first time today.
>> Howver i am a bit surprised:
>>
>>
>> avc:  denied  { module_load } for  pid=473 comm="modprobe"
>> scontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023
>> tcontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023 tclass=system
>> permissive=1
>>
>> Should that permission not have applied to a kernel module object
>> instead of "self"?
>> --
>> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
>> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
>> Dominick Grift
>>
>>
>> _______________________________________________
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.


-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 648 bytes --]

Re: should module_load be checked on a kernel module object?

[Jeffrey Vander Stoep]

No problem. We went through a number of iterations on this patch
because of how confusing the target object for init_module is.

On Android we neverallow use of init_module. Forcing userspace to use
finit_module allows us to enforce restrictions on kernel module
origin. We only allow module loading from verified-boot protected
partitions.

https://android-review.googlesource.com/#/c/214021/

On Mon, Oct 10, 2016 at 8:10 AM, Dominick Grift <dac.override@gmail.com> wrote:
> On 10/10/2016 05:02 PM, Jeffrey Vander Stoep wrote:
>> When loading a kernel module using init_module the module is copied
>> from memory of the calling process. In that case, the target really is
>> the calling process. When using finit_module a file is passed to the
>> kernel and that file is the target object.
>>
>> See the commit message that added module_load for a more complete
>> description: https://marc.info/?l=selinux&m=145988689809307&w=2
>>
>
> Thanks, Sorry about that.
>
>> On Sun, Oct 9, 2016 at 1:10 AM, Dominick Grift <dac.override@gmail.com> wrote:
>>>
>>> I encountered a system module_load event for the first time today.
>>> Howver i am a bit surprised:
>>>
>>>
>>> avc:  denied  { module_load } for  pid=473 comm="modprobe"
>>> scontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023
>>> tcontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023 tclass=system
>>> permissive=1
>>>
>>> Should that permission not have applied to a kernel module object
>>> instead of "self"?
>>> --
>>> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
>>> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
>>> Dominick Grift
>>>
>>>
>>> _______________________________________________
>>> Selinux mailing list
>>> Selinux@tycho.nsa.gov
>>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
>
> --
> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
> Dominick Grift
>

Re: should module_load be checked on a kernel module object?

[Dominick Grift]

[-- Attachment #1.1: Type: text/plain, Size: 2789 bytes --]

On 10/10/2016 07:16 PM, Jeffrey Vander Stoep wrote:
> No problem. We went through a number of iterations on this patch
> because of how confusing the target object for init_module is.
> 
> On Android we neverallow use of init_module. Forcing userspace to use
> finit_module allows us to enforce restrictions on kernel module
> origin. We only allow module loading from verified-boot protected
> partitions.
> 
> https://android-review.googlesource.com/#/c/214021/
> 

That is a nice approach. After you reminded me, i started looking at my
policy and i actually commented it (i rarely comment in my policy):

		; for compatibility with Linux =< 4.6
		(allow sys.load_kernel_module_subj_type_attribute self
(system (module_load))))))

So i suppose if i want to support Linux 4.6 then i might not have the
option to neverallow it.

> On Mon, Oct 10, 2016 at 8:10 AM, Dominick Grift <dac.override@gmail.com> wrote:
>> On 10/10/2016 05:02 PM, Jeffrey Vander Stoep wrote:
>>> When loading a kernel module using init_module the module is copied
>>> from memory of the calling process. In that case, the target really is
>>> the calling process. When using finit_module a file is passed to the
>>> kernel and that file is the target object.
>>>
>>> See the commit message that added module_load for a more complete
>>> description: https://marc.info/?l=selinux&m=145988689809307&w=2
>>>
>>
>> Thanks, Sorry about that.
>>
>>> On Sun, Oct 9, 2016 at 1:10 AM, Dominick Grift <dac.override@gmail.com> wrote:
>>>>
>>>> I encountered a system module_load event for the first time today.
>>>> Howver i am a bit surprised:
>>>>
>>>>
>>>> avc:  denied  { module_load } for  pid=473 comm="modprobe"
>>>> scontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023
>>>> tcontext=wheel.id:sysadm.role:lmc.subj:s0-s0:c0.c1023 tclass=system
>>>> permissive=1
>>>>
>>>> Should that permission not have applied to a kernel module object
>>>> instead of "self"?
>>>> --
>>>> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
>>>> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
>>>> Dominick Grift
>>>>
>>>>
>>>> _______________________________________________
>>>> Selinux mailing list
>>>> Selinux@tycho.nsa.gov
>>>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>>>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>
>>
>> --
>> Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
>> https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
>> Dominick Grift
>>


-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 648 bytes --]

Re: should module_load be checked on a kernel module object?

[Jeffrey Vander Stoep]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

On Mon, Oct 10, 2016 at 10:21 AM Dominick Grift <dac.override@gmail.com>
wrote:

> On 10/10/2016 07:16 PM, Jeffrey Vander Stoep wrote:
> > No problem. We went through a number of iterations on this patch
> > because of how confusing the target object for init_module is.
> >
> > On Android we neverallow use of init_module. Forcing userspace to use
> > finit_module allows us to enforce restrictions on kernel module
> > origin. We only allow module loading from verified-boot protected
> > partitions.
> >
> > https://android-review.googlesource.com/#/c/214021/
> >
>
> That is a nice approach. After you reminded me, i started looking at my
> policy and i actually commented it (i rarely comment in my policy):
>
>                 ; for compatibility with Linux =< 4.6
>                 (allow sys.load_kernel_module_subj_type_attribute self
> (system (module_load))))))
>
> So i suppose if i want to support Linux 4.6 then i might not have the
> option to neverallow it.
>
>
You shouldn't need this for compatibility. For kernel version <= 4.6, the
kernel hook for selinux_kernel_read_file is unused so no policy is needed,
it will already be allowed (or rather, not checked).

The issue is that modprobe uses init_module() to load a kernel module. That
would need to be updated to use finit_module() in order to disallow
init_module().

modprobe could be updated to behave more like insmod which defaults to
using finit_module and falls back to init_module for old kernels.
https://android.googlesource.com/platform/external/toybox/+/android-7.0.0_r14/toys/other/insmod.c#37

I don't know what kind of control you have over kernels, but if you want a
stable backport of the module_load patch, we backported to 4.4, 4,1, 3.18,
3.14, and 3.10:
https://android-review.googlesource.com/#/q/61d612ea731e57dc510472fb746b55cdc017f371+owner:jeffv

[-- Attachment #2: Type: text/html, Size: 3727 bytes --]

Re: should module_load be checked on a kernel module object?

[Dominick Grift]

[-- Attachment #1.1: Type: text/plain, Size: 2498 bytes --]

On 10/10/2016 07:50 PM, Jeffrey Vander Stoep wrote:
> On Mon, Oct 10, 2016 at 10:21 AM Dominick Grift <dac.override@gmail.com>
> wrote:
> 
>> On 10/10/2016 07:16 PM, Jeffrey Vander Stoep wrote:
>>> No problem. We went through a number of iterations on this patch
>>> because of how confusing the target object for init_module is.
>>>
>>> On Android we neverallow use of init_module. Forcing userspace to use
>>> finit_module allows us to enforce restrictions on kernel module
>>> origin. We only allow module loading from verified-boot protected
>>> partitions.
>>>
>>> https://android-review.googlesource.com/#/c/214021/
>>>
>>
>> That is a nice approach. After you reminded me, i started looking at my
>> policy and i actually commented it (i rarely comment in my policy):
>>
>>                 ; for compatibility with Linux =< 4.6
>>                 (allow sys.load_kernel_module_subj_type_attribute self
>> (system (module_load))))))
>>
>> So i suppose if i want to support Linux 4.6 then i might not have the
>> option to neverallow it.
>>
>>
> You shouldn't need this for compatibility. For kernel version <= 4.6, the
> kernel hook for selinux_kernel_read_file is unused so no policy is needed,
> it will already be allowed (or rather, not checked).
> 
> The issue is that modprobe uses init_module() to load a kernel module. That
> would need to be updated to use finit_module() in order to disallow
> init_module().
> 
> modprobe could be updated to behave more like insmod which defaults to
> using finit_module and falls back to init_module for old kernels.
> https://android.googlesource.com/platform/external/toybox/+/android-7.0.0_r14/toys/other/insmod.c#37
> 
> I don't know what kind of control you have over kernels, but if you want a
> stable backport of the module_load patch, we backported to 4.4, 4,1, 3.18,
> 3.14, and 3.10:
> https://android-review.googlesource.com/#/q/61d612ea731e57dc510472fb746b55cdc017f371+owner:jeffv
> 

Thank you. All is clear now. I will rephrase my comment and next time
look before asking because, even though the comment is inaccurate it,
together with the av allow rule still clearly indicated that this event
was to be expected (simply because in GNU/Linux some components use
init_module() and dont fall back to finit_module()

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 648 bytes --]

Re: should module_load be checked on a kernel module object?

[Dominick Grift]

[-- Attachment #1.1: Type: text/plain, Size: 3343 bytes --]

On 10/10/2016 07:56 PM, Dominick Grift wrote:
> On 10/10/2016 07:50 PM, Jeffrey Vander Stoep wrote:
>> On Mon, Oct 10, 2016 at 10:21 AM Dominick Grift <dac.override@gmail.com>
>> wrote:
>>
>>> On 10/10/2016 07:16 PM, Jeffrey Vander Stoep wrote:
>>>> No problem. We went through a number of iterations on this patch
>>>> because of how confusing the target object for init_module is.
>>>>
>>>> On Android we neverallow use of init_module. Forcing userspace to use
>>>> finit_module allows us to enforce restrictions on kernel module
>>>> origin. We only allow module loading from verified-boot protected
>>>> partitions.
>>>>
>>>> https://android-review.googlesource.com/#/c/214021/
>>>>
>>>
>>> That is a nice approach. After you reminded me, i started looking at my
>>> policy and i actually commented it (i rarely comment in my policy):
>>>
>>>                 ; for compatibility with Linux =< 4.6
>>>                 (allow sys.load_kernel_module_subj_type_attribute self
>>> (system (module_load))))))
>>>
>>> So i suppose if i want to support Linux 4.6 then i might not have the
>>> option to neverallow it.
>>>
>>>
>> You shouldn't need this for compatibility. For kernel version <= 4.6, the
>> kernel hook for selinux_kernel_read_file is unused so no policy is needed,
>> it will already be allowed (or rather, not checked).
>>
>> The issue is that modprobe uses init_module() to load a kernel module. That
>> would need to be updated to use finit_module() in order to disallow
>> init_module().
>>
>> modprobe could be updated to behave more like insmod which defaults to
>> using finit_module and falls back to init_module for old kernels.
>> https://android.googlesource.com/platform/external/toybox/+/android-7.0.0_r14/toys/other/insmod.c#37
>>
>> I don't know what kind of control you have over kernels, but if you want a
>> stable backport of the module_load patch, we backported to 4.4, 4,1, 3.18,
>> 3.14, and 3.10:
>> https://android-review.googlesource.com/#/q/61d612ea731e57dc510472fb746b55cdc017f371+owner:jeffv
>>
> 
> Thank you. All is clear now. I will rephrase my comment and next time
> look before asking because, even though the comment is inaccurate it,
> together with the av allow rule still clearly indicated that this event
> was to be expected (simply because in GNU/Linux some components use
> init_module() and dont fall back to finit_module()
> 

After some deliberation i decided to conditionally support init_module()

It seems finit_module() requires that kernel modules aren't compressed.
Distributions compress kernel modules to save valuable space, and thus
even though kmod tries to use finit_module() by default it falls back to
init_module() since the modules are compressed.

By conditionally supporting init_module() one can make it work by
default but give one the opportunity to enable finit_module by
decompressing kernel modules and by toggling the boolean that allows
init_module() to false.


https://github.com/DefenSec/dssp/commit/12994152cce90a5cfecd7dbbc2ffb1c1d524ad98

https://github.com/DefenSec/dssp/commit/7f31dffeb2af83a23d2f47dd7cfbf6f406a5d243
-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 648 bytes --]