From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753770AbdK3Aou (ORCPT ); Wed, 29 Nov 2017 19:44:50 -0500 Received: from mail-ua0-f194.google.com ([209.85.217.194]:45679 "EHLO mail-ua0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753749AbdK3Aor (ORCPT ); Wed, 29 Nov 2017 19:44:47 -0500 X-Google-Smtp-Source: AGs4zMYyB09VI0vbd7anCtkMGVnARn52i484YV4tvJRCox/nbT8jJH8APH4fwUUqieECRWNu5Cb6HIp2fJC5PnhAJEE= MIME-Version: 1.0 In-Reply-To: References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> <1511803118-2552-6-git-send-email-tixxdz@gmail.com> <1100603534.56586.1511871419952@ichabod.co-bxl> <20171128193243.4fymnjk7fplqw62x@thunk.org> <708003731.69563.1511905898471@ichabod.co-bxl> From: Kees Cook Date: Wed, 29 Nov 2017 16:44:45 -0800 X-Google-Sender-Auth: T12ac3RLYngWd2iIrYdoors_CoY Message-ID: Subject: Re: [kernel-hardening] Re: [PATCH v5 next 5/5] net: modules: use request_module_cap() to load 'netdev-%s' modules To: Linus Torvalds Cc: Djalal Harouni , Jessica Yu , LSM List , Linux Kernel Mailing List , "kernel-hardening@lists.openwall.com" 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 Wed, Nov 29, 2017 at 2:14 PM, Linus Torvalds wrote: > On Wed, Nov 29, 2017 at 1:17 PM, Kees Cook wrote: >> 1) Add request_module_cap(required_cap, module_name_prefix, fmt, fmt_args...) >> >> 2) Convert known privileged-but-not-CAP_SYS_MODULE request_module() >> callers to request_module_cap(the_needed_cap, prefix, ...) > > Yes. The upside seems to be very limited here, but at least it makes > the users that use CAP_NET_ADMIN instead of CAP_SYS_MODULE able to > specify so. Yeah, agreed; it's limited in scope. >> 2) Convert known unprivileged callers to use request_module_cap(0, ...) > > 0 is CAP_CHOWN, so it would have to be -1. Oops, yes, think-o. > And I wouldn't actually want to see that as-is. Not only would I not > want to see people have that "-1" in random driver subsystems, I'd > much prefer to have actual helper naming that descibes why something > is ok So, if some catch-all is needed, maybe request_module_unpriv(...), but I think it should be possible to identify SOME kind of defining privilege to check. > Because as mentioned, I think there are valid permission reasons that > are _not_ about capabilities that make you able to load a module. > > If you can open a character device node, then "misc_open()" will do > > request_module("char-major-%d-%d", MISC_MAJOR, minor); > > and there is nothing about capabilities in the CAP_SYS_MODULE sense > about the user. But the user _did_ have the privileges to open that > character device file. > > That's why I suggested something like request_module_dev(): it's not > at all the same thing as request_module_cap(-1, ...), saying "I don't > need/have a capability". It's saying something else entirely, it's > basically saying "I have the right based on device permissions". > > And something like request_module_dev() might even have real semantic > meaning, exactly because it says "this module request comes from > trying to open a device node". I just want to make sure I'm visualizing this correctly. Using the misc_open() example, you're thinking something like: request_module_dev(file, "char-major-%d-%d", MISC_MAJOR, minor); which would then do a verification that file->f_cred matched current_cred()? (For misc_open(), this is obviously going to always be okay, but other cases may be further from the fops open handler...) > Why would that be? If we know we're on a system where /dev is > auto-populated through udev, then the device nodes should have been > created by the drivers, not the other way around. So we might even > have a rule that notices that automatically, and simply disables > request_module_dev() entirely. Right, we have both directions -- hotplug module loading ("I saw this PCI alias, load something! Oh! It needs some fancy crypto too!") and on-use module loading. It seems like on-use module loading would get covered by request_module_dev(), and the hotplug case would always be privileged. Anyway... exploring this with real code is clearly warranted. > I suspect that for a lot of our existing request_module() cases, they > really are pretty trivial. In most cases, it's probably really about > whether you have the hardware or not. Right, and those would all be privileged, etc. > So for the hardware driver cases, either the hardware enumerates > itself, or it is presumably set up by the system scripts anyway, and > CAP_SYS_MODULE is all fine. The "open device node" case is one special > case, though. Yeah, going through the call sites and asking "where does the privilege for loading this module derive from?" for each one will help shape the resulting API changes. > That mainly leaves the protocol ones we need to look out for, I suspect. This is where a lot of the exposure really comes from. socket() triggers a bunch of stuff, but doesn't have an obvious privilege associated with it... while it already does the name templates, maybe add request_module_socket() just to explicitly mark it? >> 3) Add WARN_RATELIMIT for request_module() calls without >> CAP_SYS_MODULE to shake out other places where request_module_cap() is >> needed. > > Yes. > > And this is where I hope that there really aren't actually all that > many cases that will warn, and that it's hopefully easy to simply just > look at a handful of reports and say "ok, that case is obviously > fine". > > And I may be wrong. > >> 4) Adapt the original patch series to add the per-process flag that >> can block privilege elevations. > > Yes. Okay, excellent. Hopefully we haven't scared off Djalal, and hopefully Jessica doesn't think this is all insane. :) Thanks! -Kees -- Kees Cook Pixel Security From mboxrd@z Thu Jan 1 00:00:00 1970 From: keescook@chromium.org (Kees Cook) Date: Wed, 29 Nov 2017 16:44:45 -0800 Subject: [kernel-hardening] Re: [PATCH v5 next 5/5] net: modules: use request_module_cap() to load 'netdev-%s' modules In-Reply-To: References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> <1511803118-2552-6-git-send-email-tixxdz@gmail.com> <1100603534.56586.1511871419952@ichabod.co-bxl> <20171128193243.4fymnjk7fplqw62x@thunk.org> <708003731.69563.1511905898471@ichabod.co-bxl> Message-ID: To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Wed, Nov 29, 2017 at 2:14 PM, Linus Torvalds wrote: > On Wed, Nov 29, 2017 at 1:17 PM, Kees Cook wrote: >> 1) Add request_module_cap(required_cap, module_name_prefix, fmt, fmt_args...) >> >> 2) Convert known privileged-but-not-CAP_SYS_MODULE request_module() >> callers to request_module_cap(the_needed_cap, prefix, ...) > > Yes. The upside seems to be very limited here, but at least it makes > the users that use CAP_NET_ADMIN instead of CAP_SYS_MODULE able to > specify so. Yeah, agreed; it's limited in scope. >> 2) Convert known unprivileged callers to use request_module_cap(0, ...) > > 0 is CAP_CHOWN, so it would have to be -1. Oops, yes, think-o. > And I wouldn't actually want to see that as-is. Not only would I not > want to see people have that "-1" in random driver subsystems, I'd > much prefer to have actual helper naming that descibes why something > is ok So, if some catch-all is needed, maybe request_module_unpriv(...), but I think it should be possible to identify SOME kind of defining privilege to check. > Because as mentioned, I think there are valid permission reasons that > are _not_ about capabilities that make you able to load a module. > > If you can open a character device node, then "misc_open()" will do > > request_module("char-major-%d-%d", MISC_MAJOR, minor); > > and there is nothing about capabilities in the CAP_SYS_MODULE sense > about the user. But the user _did_ have the privileges to open that > character device file. > > That's why I suggested something like request_module_dev(): it's not > at all the same thing as request_module_cap(-1, ...), saying "I don't > need/have a capability". It's saying something else entirely, it's > basically saying "I have the right based on device permissions". > > And something like request_module_dev() might even have real semantic > meaning, exactly because it says "this module request comes from > trying to open a device node". I just want to make sure I'm visualizing this correctly. Using the misc_open() example, you're thinking something like: request_module_dev(file, "char-major-%d-%d", MISC_MAJOR, minor); which would then do a verification that file->f_cred matched current_cred()? (For misc_open(), this is obviously going to always be okay, but other cases may be further from the fops open handler...) > Why would that be? If we know we're on a system where /dev is > auto-populated through udev, then the device nodes should have been > created by the drivers, not the other way around. So we might even > have a rule that notices that automatically, and simply disables > request_module_dev() entirely. Right, we have both directions -- hotplug module loading ("I saw this PCI alias, load something! Oh! It needs some fancy crypto too!") and on-use module loading. It seems like on-use module loading would get covered by request_module_dev(), and the hotplug case would always be privileged. Anyway... exploring this with real code is clearly warranted. > I suspect that for a lot of our existing request_module() cases, they > really are pretty trivial. In most cases, it's probably really about > whether you have the hardware or not. Right, and those would all be privileged, etc. > So for the hardware driver cases, either the hardware enumerates > itself, or it is presumably set up by the system scripts anyway, and > CAP_SYS_MODULE is all fine. The "open device node" case is one special > case, though. Yeah, going through the call sites and asking "where does the privilege for loading this module derive from?" for each one will help shape the resulting API changes. > That mainly leaves the protocol ones we need to look out for, I suspect. This is where a lot of the exposure really comes from. socket() triggers a bunch of stuff, but doesn't have an obvious privilege associated with it... while it already does the name templates, maybe add request_module_socket() just to explicitly mark it? >> 3) Add WARN_RATELIMIT for request_module() calls without >> CAP_SYS_MODULE to shake out other places where request_module_cap() is >> needed. > > Yes. > > And this is where I hope that there really aren't actually all that > many cases that will warn, and that it's hopefully easy to simply just > look at a handful of reports and say "ok, that case is obviously > fine". > > And I may be wrong. > >> 4) Adapt the original patch series to add the per-process flag that >> can block privilege elevations. > > Yes. Okay, excellent. Hopefully we haven't scared off Djalal, and hopefully Jessica doesn't think this is all insane. :) Thanks! -Kees -- Kees Cook Pixel Security -- 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 Sender: keescook@google.com In-Reply-To: References: <1511803118-2552-1-git-send-email-tixxdz@gmail.com> <1511803118-2552-6-git-send-email-tixxdz@gmail.com> <1100603534.56586.1511871419952@ichabod.co-bxl> <20171128193243.4fymnjk7fplqw62x@thunk.org> <708003731.69563.1511905898471@ichabod.co-bxl> From: Kees Cook Date: Wed, 29 Nov 2017 16:44:45 -0800 Message-ID: Content-Type: text/plain; charset="UTF-8" Subject: Re: [kernel-hardening] Re: [PATCH v5 next 5/5] net: modules: use request_module_cap() to load 'netdev-%s' modules To: Linus Torvalds Cc: Djalal Harouni , Jessica Yu , LSM List , Linux Kernel Mailing List , "kernel-hardening@lists.openwall.com" List-ID: On Wed, Nov 29, 2017 at 2:14 PM, Linus Torvalds wrote: > On Wed, Nov 29, 2017 at 1:17 PM, Kees Cook wrote: >> 1) Add request_module_cap(required_cap, module_name_prefix, fmt, fmt_args...) >> >> 2) Convert known privileged-but-not-CAP_SYS_MODULE request_module() >> callers to request_module_cap(the_needed_cap, prefix, ...) > > Yes. The upside seems to be very limited here, but at least it makes > the users that use CAP_NET_ADMIN instead of CAP_SYS_MODULE able to > specify so. Yeah, agreed; it's limited in scope. >> 2) Convert known unprivileged callers to use request_module_cap(0, ...) > > 0 is CAP_CHOWN, so it would have to be -1. Oops, yes, think-o. > And I wouldn't actually want to see that as-is. Not only would I not > want to see people have that "-1" in random driver subsystems, I'd > much prefer to have actual helper naming that descibes why something > is ok So, if some catch-all is needed, maybe request_module_unpriv(...), but I think it should be possible to identify SOME kind of defining privilege to check. > Because as mentioned, I think there are valid permission reasons that > are _not_ about capabilities that make you able to load a module. > > If you can open a character device node, then "misc_open()" will do > > request_module("char-major-%d-%d", MISC_MAJOR, minor); > > and there is nothing about capabilities in the CAP_SYS_MODULE sense > about the user. But the user _did_ have the privileges to open that > character device file. > > That's why I suggested something like request_module_dev(): it's not > at all the same thing as request_module_cap(-1, ...), saying "I don't > need/have a capability". It's saying something else entirely, it's > basically saying "I have the right based on device permissions". > > And something like request_module_dev() might even have real semantic > meaning, exactly because it says "this module request comes from > trying to open a device node". I just want to make sure I'm visualizing this correctly. Using the misc_open() example, you're thinking something like: request_module_dev(file, "char-major-%d-%d", MISC_MAJOR, minor); which would then do a verification that file->f_cred matched current_cred()? (For misc_open(), this is obviously going to always be okay, but other cases may be further from the fops open handler...) > Why would that be? If we know we're on a system where /dev is > auto-populated through udev, then the device nodes should have been > created by the drivers, not the other way around. So we might even > have a rule that notices that automatically, and simply disables > request_module_dev() entirely. Right, we have both directions -- hotplug module loading ("I saw this PCI alias, load something! Oh! It needs some fancy crypto too!") and on-use module loading. It seems like on-use module loading would get covered by request_module_dev(), and the hotplug case would always be privileged. Anyway... exploring this with real code is clearly warranted. > I suspect that for a lot of our existing request_module() cases, they > really are pretty trivial. In most cases, it's probably really about > whether you have the hardware or not. Right, and those would all be privileged, etc. > So for the hardware driver cases, either the hardware enumerates > itself, or it is presumably set up by the system scripts anyway, and > CAP_SYS_MODULE is all fine. The "open device node" case is one special > case, though. Yeah, going through the call sites and asking "where does the privilege for loading this module derive from?" for each one will help shape the resulting API changes. > That mainly leaves the protocol ones we need to look out for, I suspect. This is where a lot of the exposure really comes from. socket() triggers a bunch of stuff, but doesn't have an obvious privilege associated with it... while it already does the name templates, maybe add request_module_socket() just to explicitly mark it? >> 3) Add WARN_RATELIMIT for request_module() calls without >> CAP_SYS_MODULE to shake out other places where request_module_cap() is >> needed. > > Yes. > > And this is where I hope that there really aren't actually all that > many cases that will warn, and that it's hopefully easy to simply just > look at a handful of reports and say "ok, that case is obviously > fine". > > And I may be wrong. > >> 4) Adapt the original patch series to add the per-process flag that >> can block privilege elevations. > > Yes. Okay, excellent. Hopefully we haven't scared off Djalal, and hopefully Jessica doesn't think this is all insane. :) Thanks! -Kees -- Kees Cook Pixel Security