From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752713AbdK3MWK (ORCPT ); Thu, 30 Nov 2017 07:22:10 -0500 Received: from mail-qt0-f194.google.com ([209.85.216.194]:34067 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751157AbdK3MWG (ORCPT ); Thu, 30 Nov 2017 07:22:06 -0500 X-Google-Smtp-Source: AGs4zMahfsNTCQBWJPT8B4kDoymA0wAaimNUpmwzI7x2qmzXVgDWAeUovPOiSYBl9O4J4gYD1WA8JKgXoyOxOJJocCg= MIME-Version: 1.0 In-Reply-To: <20171130012324.GZ729@wotan.suse.de> References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> <1511803118-2552-4-git-send-email-tixxdz@gmail.com> <20171130012324.GZ729@wotan.suse.de> From: Djalal Harouni Date: Thu, 30 Nov 2017 13:22:05 +0100 Message-ID: Subject: Re: [PATCH v5 next 3/5] modules:capabilities: automatic module loading restriction To: "Luis R. Rodriguez" Cc: Kees Cook , Andy Lutomirski , Andrew Morton , James Morris , Ben Hutchings , Solar Designer , Serge Hallyn , Jessica Yu , Rusty Russell , linux-kernel , LSM List , kernel-hardening@lists.openwall.com, Jonathan Corbet , Ingo Molnar , "David S. Miller" , Network Development , Peter Zijlstra , Linus Torvalds Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 30, 2017 at 2:23 AM, Luis R. Rodriguez wrote: > On Mon, Nov 27, 2017 at 06:18:36PM +0100, Djalal Harouni wrote: >> diff --git a/include/linux/module.h b/include/linux/module.h >> index 5cbb239..c36aed8 100644 >> --- a/include/linux/module.h >> +++ b/include/linux/module.h >> @@ -261,7 +261,16 @@ struct notifier_block; >> >> #ifdef CONFIG_MODULES >> >> -extern int modules_disabled; /* for sysctl */ >> +enum { >> + MODULES_AUTOLOAD_ALLOWED = 0, >> + MODULES_AUTOLOAD_PRIVILEGED = 1, >> + MODULES_AUTOLOAD_DISABLED = 2, >> +}; >> + > > Can you kdocify these and add a respective rst doc file? Maybe stuff your > extensive docs which you are currently adding to > Documentation/sysctl/kernel.txt to this new file and in kernel.txt just refer > to it. This way this can be also nicely visibly documented on the web with the > new sphinx. > > This way you can take advantage of the kdocs you are already adding and refer > to them. Alright I'll do it in the next series next week, we'll change the semantics as requested by Linus and Kees here: http://www.openwall.com/lists/kernel-hardening/2017/11/29/38 To block the privilege escalation through the usermod helper. >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 2fb4e27..0b6f0c8 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -683,6 +688,15 @@ static struct ctl_table kern_table[] = { >> .extra1 = &one, >> .extra2 = &one, >> }, >> + { >> + .procname = "modules_autoload_mode", >> + .data = &modules_autoload_mode, >> + .maxlen = sizeof(int), >> + .mode = 0644, >> + .proc_handler = modules_autoload_dointvec_minmax, > > It would seem this is a unint ... so why not reflect that? > >> @@ -2499,6 +2513,20 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, >> } >> #endif >> >> +#ifdef CONFIG_MODULES >> +static int modules_autoload_dointvec_minmax(struct ctl_table *table, int write, >> + void __user *buffer, size_t *lenp, loff_t *ppos) >> +{ >> + /* >> + * Only CAP_SYS_MODULE in init user namespace are allowed to change this >> + */ >> + if (write && !capable(CAP_SYS_MODULE)) >> + return -EPERM; >> + >> + return proc_dointvec_minmax(table, write, buffer, lenp, ppos); >> +} >> +#endif > > We now have proc_douintvec_minmax(). > Yes, however in that same response by Linus it was suggested to drop the sysctl completely, so next iterations will not have this code. Thank you for the review! -- tixxdz From mboxrd@z Thu Jan 1 00:00:00 1970 From: tixxdz@gmail.com (Djalal Harouni) Date: Thu, 30 Nov 2017 13:22:05 +0100 Subject: [PATCH v5 next 3/5] modules:capabilities: automatic module loading restriction In-Reply-To: <20171130012324.GZ729@wotan.suse.de> References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> <1511803118-2552-4-git-send-email-tixxdz@gmail.com> <20171130012324.GZ729@wotan.suse.de> Message-ID: To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Thu, Nov 30, 2017 at 2:23 AM, Luis R. Rodriguez wrote: > On Mon, Nov 27, 2017 at 06:18:36PM +0100, Djalal Harouni wrote: >> diff --git a/include/linux/module.h b/include/linux/module.h >> index 5cbb239..c36aed8 100644 >> --- a/include/linux/module.h >> +++ b/include/linux/module.h >> @@ -261,7 +261,16 @@ struct notifier_block; >> >> #ifdef CONFIG_MODULES >> >> -extern int modules_disabled; /* for sysctl */ >> +enum { >> + MODULES_AUTOLOAD_ALLOWED = 0, >> + MODULES_AUTOLOAD_PRIVILEGED = 1, >> + MODULES_AUTOLOAD_DISABLED = 2, >> +}; >> + > > Can you kdocify these and add a respective rst doc file? Maybe stuff your > extensive docs which you are currently adding to > Documentation/sysctl/kernel.txt to this new file and in kernel.txt just refer > to it. This way this can be also nicely visibly documented on the web with the > new sphinx. > > This way you can take advantage of the kdocs you are already adding and refer > to them. Alright I'll do it in the next series next week, we'll change the semantics as requested by Linus and Kees here: http://www.openwall.com/lists/kernel-hardening/2017/11/29/38 To block the privilege escalation through the usermod helper. >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 2fb4e27..0b6f0c8 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -683,6 +688,15 @@ static struct ctl_table kern_table[] = { >> .extra1 = &one, >> .extra2 = &one, >> }, >> + { >> + .procname = "modules_autoload_mode", >> + .data = &modules_autoload_mode, >> + .maxlen = sizeof(int), >> + .mode = 0644, >> + .proc_handler = modules_autoload_dointvec_minmax, > > It would seem this is a unint ... so why not reflect that? > >> @@ -2499,6 +2513,20 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, >> } >> #endif >> >> +#ifdef CONFIG_MODULES >> +static int modules_autoload_dointvec_minmax(struct ctl_table *table, int write, >> + void __user *buffer, size_t *lenp, loff_t *ppos) >> +{ >> + /* >> + * Only CAP_SYS_MODULE in init user namespace are allowed to change this >> + */ >> + if (write && !capable(CAP_SYS_MODULE)) >> + return -EPERM; >> + >> + return proc_dointvec_minmax(table, write, buffer, lenp, ppos); >> +} >> +#endif > > We now have proc_douintvec_minmax(). > Yes, however in that same response by Linus it was suggested to drop the sysctl completely, so next iterations will not have this code. Thank you for the review! -- tixxdz -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 MIME-Version: 1.0 In-Reply-To: <20171130012324.GZ729@wotan.suse.de> References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> <1511803118-2552-4-git-send-email-tixxdz@gmail.com> <20171130012324.GZ729@wotan.suse.de> From: Djalal Harouni Date: Thu, 30 Nov 2017 13:22:05 +0100 Message-ID: Content-Type: text/plain; charset="UTF-8" Subject: [kernel-hardening] Re: [PATCH v5 next 3/5] modules:capabilities: automatic module loading restriction To: "Luis R. Rodriguez" Cc: Kees Cook , Andy Lutomirski , Andrew Morton , James Morris , Ben Hutchings , Solar Designer , Serge Hallyn , Jessica Yu , Rusty Russell , linux-kernel , LSM List , kernel-hardening@lists.openwall.com, Jonathan Corbet , Ingo Molnar , "David S. Miller" , Network Development , Peter Zijlstra , Linus Torvalds List-ID: On Thu, Nov 30, 2017 at 2:23 AM, Luis R. Rodriguez wrote: > On Mon, Nov 27, 2017 at 06:18:36PM +0100, Djalal Harouni wrote: >> diff --git a/include/linux/module.h b/include/linux/module.h >> index 5cbb239..c36aed8 100644 >> --- a/include/linux/module.h >> +++ b/include/linux/module.h >> @@ -261,7 +261,16 @@ struct notifier_block; >> >> #ifdef CONFIG_MODULES >> >> -extern int modules_disabled; /* for sysctl */ >> +enum { >> + MODULES_AUTOLOAD_ALLOWED = 0, >> + MODULES_AUTOLOAD_PRIVILEGED = 1, >> + MODULES_AUTOLOAD_DISABLED = 2, >> +}; >> + > > Can you kdocify these and add a respective rst doc file? Maybe stuff your > extensive docs which you are currently adding to > Documentation/sysctl/kernel.txt to this new file and in kernel.txt just refer > to it. This way this can be also nicely visibly documented on the web with the > new sphinx. > > This way you can take advantage of the kdocs you are already adding and refer > to them. Alright I'll do it in the next series next week, we'll change the semantics as requested by Linus and Kees here: http://www.openwall.com/lists/kernel-hardening/2017/11/29/38 To block the privilege escalation through the usermod helper. >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 2fb4e27..0b6f0c8 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -683,6 +688,15 @@ static struct ctl_table kern_table[] = { >> .extra1 = &one, >> .extra2 = &one, >> }, >> + { >> + .procname = "modules_autoload_mode", >> + .data = &modules_autoload_mode, >> + .maxlen = sizeof(int), >> + .mode = 0644, >> + .proc_handler = modules_autoload_dointvec_minmax, > > It would seem this is a unint ... so why not reflect that? > >> @@ -2499,6 +2513,20 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, >> } >> #endif >> >> +#ifdef CONFIG_MODULES >> +static int modules_autoload_dointvec_minmax(struct ctl_table *table, int write, >> + void __user *buffer, size_t *lenp, loff_t *ppos) >> +{ >> + /* >> + * Only CAP_SYS_MODULE in init user namespace are allowed to change this >> + */ >> + if (write && !capable(CAP_SYS_MODULE)) >> + return -EPERM; >> + >> + return proc_dointvec_minmax(table, write, buffer, lenp, ppos); >> +} >> +#endif > > We now have proc_douintvec_minmax(). > Yes, however in that same response by Linus it was suggested to drop the sysctl completely, so next iterations will not have this code. Thank you for the review! -- tixxdz