From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1435063AbdDZJZo (ORCPT ); Wed, 26 Apr 2017 05:25:44 -0400 Received: from mail-qt0-f196.google.com ([209.85.216.196]:34289 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965130AbdDZJGO (ORCPT ); Wed, 26 Apr 2017 05:06:14 -0400 MIME-Version: 1.0 In-Reply-To: <87r30ifmwz.fsf@rustcorp.com.au> References: <1492640420-27345-1-git-send-email-tixxdz@gmail.com> <1492640420-27345-3-git-send-email-tixxdz@gmail.com> <87r30ifmwz.fsf@rustcorp.com.au> From: Djalal Harouni Date: Wed, 26 Apr 2017 11:06:12 +0200 Message-ID: Subject: Re: [PATCH v3 2/2] modules:capabilities: add a per-task modules autoload restriction To: Rusty Russell Cc: Linux Kernel Mailing List , Andy Lutomirski , Kees Cook , Andrew Morton , "Serge E. Hallyn" , kernel-hardening@lists.openwall.com, LSM List , Linux API , Dongsu Park , Casey Schaufler , James Morris , Paul Moore , Tetsuo Handa , Greg Kroah-Hartman , Jonathan Corbet , Jessica Yu , Arnaldo Carvalho de Melo , Mauro Carvalho Chehab , Ingo Molnar , Zendyani , Peter Zijlstra Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rusty, On Mon, Apr 24, 2017 at 6:29 AM, Rusty Russell wrote: > Djalal Harouni writes: >> When value is (1), task must have CAP_SYS_MODULE to be able to trigger a >> module auto-load operation, or CAP_NET_ADMIN for modules with a >> 'netdev-%s' alias. > > Sorry, the magic 'netdev-' prefix is a crawling horror. To do this Yes I agree, that's the not the best part. I added it for backward compatibility since I did notice that some network daemon retain CAP_NET_ADMIN for this case. The aim of the patches is to get modules autoload covered with CAP_SYS_MODULE and make it more like explicit modules loading. > properly, you need to hand the capability (if any) from the > request_module() call. Probably by adding a new request_module_cap and > making request_module() call that, then fixing up the callers. Hmm, sorry Rusty I'm a bit confused, when you refer to "callers", you mean request_module() callers ? If so, I'm not sure that the best thing here since it may defeat the purpose of this feature if we allow to pass capabilities. Right now we have modules autoload that can be triggered without privileges, or with CAP_SYS_ADMIN, CAP_NET_ADMIN, CAP_SYS_MODULE... and some caps may allow to load other modules to resolve symbols etc. The public exploits did target CAP_NET_ADMIN, if we offer a way to pass in capabilities it will be used it as it is the case now without it, not to mention that some capabilities are overloaded, exploits will continue for these ones. The goal is to narrow modules autoload only to CAP_SYS_MODULE or disable it completely for process trees. Later you can give CAP_SYS_MODULE and you are sure that only /lib/modules/ will be autoloaded, no arbitrary loads by using seccomp filter on module syscalls, or separate the process in two one with CAP_SYS_MODULE and the other with its own capabilities and feature use. I also don't like that 'netdev-%s' but it is for compatibility for specific cases, maybe there are others that we may have to add. But I would prefer if we narrow it down to only CAP_SYS_MODULE. How about I move all permission checks to security/commoncap.c helpers, the module logic part will only contain set and read sysctl "modules_autoload" and "task->modules_autoload" ? I already added cap_kernel_module_request() which is called by request_module(), so instead of counting on module to do the permission check I will open code it inside security/commoncap.c:cap_kernel_module_request() like other capability helpers and note that CAP_NET_ADMIN 'netdev-%s' alias is only for compatibility. This way we prevent that other capabilities get exposed or targeted for exploitation. Do you think that this will work ? Thanks! > Cheers, > Rusty. -- tixxdz