Re: Android kill capability denials

[Stephen Smalley <sds@tycho.nsa.gov>]

On 11/15/18 9:42 AM, Stephen Smalley wrote:
> On 11/15/18 8:11 AM, Ondrej Mosnacek wrote:
>> On Mon, Nov 12, 2018 at 7:56 AM Ravi Kumar <nxp.ravi@gmail.com> wrote:
>>> Hi team ,
>>>
>>> On android- with latest kernels 4.14  we are seeing some denials 
>>> which seem to be very much genuine to be address . Where kernel is 
>>> trying to kill its own  created process ( might be for maintenance) .
>>> These are seen in long Stress testing .  But  I dont see any one 
>>> adding such rule in general so the question is  do we see any risk  
>>> which made us not to add such rules ?
>>>
>>> 1.   avc: denied { kill } for pid=2432 comm="irq/66-90b6300." 
>>> capability=5 scontext=u:r:kernel:s0 tcontext=u:r:kernel:s0 
>>> tclass=capability permissive=0
>>> 2.   avc: denied { kill } for pid=69 comm="rcuop/6" capability=5 
>>> scontext=u:r:kernel:s0 tcontext=u:r:kernel:s0 tclass=capability 
>>> permissive=0
>>> 3.   avc: denied { kill } for pid=0 comm="swapper/1" capability=5 
>>> scontext=u:r:kernel:s0 tcontext=u:r:kernel:s0 tclass=capability 
>>> permissive=0
>>> 4.   avc: denied { kill } for pid=4185 comm="kworker/0:4" 
>>> capability=5 scontext=u:r:kernel:s0 tcontext=u:r:kernel:s0 
>>> tclass=capability permissive=0
>>>
>>> This is self capability any one in kernel context  should be able to 
>>> do such operations  I guess.
>>
>> The reference policy does contain a rule that allows this kind of
>> operations, see:
>> https://github.com/SELinuxProject/refpolicy/blob/master/policy/modules/kernel/kernel.te#L203 
>>
>>
>> It is also present in the Fedora policy on my system:
>>
>> $ sesearch -A -s kernel_t -t kernel_t -c capability -p kill
>> allow kernel_t kernel_t:capability { audit_control audit_write chown
>> dac_override dac_read_search fowner fsetid ipc_lock ipc_owner kill
>> lease linux_immutable mknod net_admin net_bind_service net_broadcast
>> net_raw setfcap setgid setpcap s
>> etuid sys_admin sys_boot sys_chroot sys_nice sys_pacct sys_ptrace
>> sys_rawio sys_resource sys_time sys_tty_config };
>>
>> Therefore I would say it is perfectly fine to add such rule to your
>> policy as well.
> 
> Android uses a completely different policy not based on the reference 
> policy at all, and tries to limit even the kernel and init domains to 
> least privilege.  There are a few reasons to limit even the kernel domain:
> 
> 1) To protect kernel threads and kernel usermodehelpers from being 
> tricked into misusing their privileges and ensuring that they always 
> transition to a non-kernel domain upon executing a usermodehelper.
> 
> 2) To render typical kernel exploits that just switch to the init cred 
> or otherwise rewrite their cred to use SECINITSID_KERNEL from gaining 
> full privileges.
> 
> There are no unconfined domains in Android policy.
> 
> I think we would at least want to know why the permission check is 
> occurring, as it might reflect a kernel bug.  If you can get system call 
> audit info, then you can see additional information that may be helpful.

I guess system call audit isn't relevant here since these are 
kernel-internal callers.

This still seems like a kernel bug though, because 
check_kill_permission() will return 0 without performing any capability 
or other permission checks if !si_fromuser(info).  So kernel callers 
shouldn't trigger such permission checks unless I missed something.