mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serge.hallyn@ubuntu.com>
To: Eric Paris <eparis@parisplace.org>
Cc: Vasiliy Kulikov <segoon@openwall.com>,
	linux-kernel@vger.kernel.org, mjt@tls.msk.ru, arnd@arndb.de,
	mirqus@gmail.com, netdev@vger.kernel.org,
	Ben Hutchings <bhutchings@solarflare.com>,
	David Miller <davem@davemloft.net>,
	kuznet@ms2.inr.ac.ru, pekkas@netcore.fi, jmorris@namei.org,
	yoshfuji@linux-ipv6.org, kaber@trash.net, eric.dumazet@gmail.com,
	therbert@google.com, xiaosuo@gmail.com, jesse@nicira.com,
	kees.cook@canonical.com, eugene@redhat.com,
	dan.j.rosenberg@gmail.com, akpm@linux-foundation.org,
	Greg KH <greg@kroah.com>, Stephen Smalley <sds@tycho.nsa.gov>,
	LSM List <linux-security-module@vger.kernel.org>,
	Daniel J Walsh <dwalsh@redhat.com>,
	David Howells <dhowells@redhat.com>
Subject: Re: [PATCH v2] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
Date: Thu, 24 Mar 2011 10:37:14 -0500	[thread overview]
Message-ID: <20110324153714.GB2648@peq.hallyn.com> (raw)
In-Reply-To: <AANLkTikr=qfFeaXDkiBbK-4KvKS83LzURrUj4L2TP8_u@mail.gmail.com>

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

Quoting Eric Paris (eparis@parisplace.org):
> On Tue, Mar 1, 2011 at 4:33 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
...
> This patch is causing a bit of a problem in Fedora.  The problem lies

Sorry, what exactly is the problem it is causing?  I gather it's
spitting out printks?  What exactly do the printks say?  The patch
included at bottom checks for CAP_NET_ADMIN before checking for
CAP_SYS_MODULE, so these must be cases which historically always
quietly failed, and are now hitting the 'pr_err' which this patch
adds?

If it really is the capable(CAP_SYS_ADMIN) call itself, then there
must be a bug in the no_module logic in the patch.  Every
case where it's currently checking for capable(CAP_SYS_ADMIN)
(and potentially auditing a failure) should be one where in the past
it would have (and currently it still would) spit out an audit msg
for the capable(CAP_NET_ADMIN) failure anyway.

If it's just the pr_err that's causing problems, then perhaps we
can turn it into a warn_once?

> mostly in the fact that we, by means of using SELinux, grant very very
> few domains CAP_SYS_MODULE (and we record when domains attempt to use
> this permission).  Unlike most other distros in which uid==0 is for
> all intensive purposes == CAP_FULL_SET and there is no logging of
> failures to use capabilities.  What happened is that as soon as we
> instituted this change we started getting SELinux denials because lots
> of domains (libvirt, udev, iw, NetworkManager) all the sudden started
> trying to use CAP_SYS_MODULE.  It took me a minute to make sure this
> patch was the problem because I wasn't seeing any printk messages.  I
> had to make some changes to the patch to print every time a task got
> into the CAP_SYS_MODULE case in order to get an idea what was causing
> the problem.  I found on my machine I hit the problem 3 times trying
> to load "reg", "wifi0", and "virbr0"    None of these are actual
> modules in userspace so the upcall failed.
> 
> I'm trying to figure out how I should be dealing with this.
> 
> My first idea is changing the capable(CAP_SYS_MODULE) into a
> has_capability_noaudit().  Which will not audit the access attempt.
> I'm not sure I like that since it uses the read cred, it doesn't set
> PF_SUPERPRIV, and it means we could likely miss recording a problem if
> a task is doing this intentionally...
> 
> The next idea is I guess figuring out what's causing these and fix
> them there, but I'm not certain a good way to debug it.  I know from
> our audit logs that wpa_supplicant is calling SIOCGIFINDEX which is
> causing one of these, libvirt is calling SIOCGIFFLAGS.  I'm not sure
> what udev->iw is doing to trigger it's problem.....
> 
> If the name in question is not coming from direct userspace request I
> guess I need help figuring out what is causing them....
> 
> So while this patch might not necessarily be breaking things it
> certainly is not regression free and could potentially be breaking
> systems with fine grained capabilities controls....
> 
> -Eric
> 
> >    root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) --
> >    root@albatros:~# grep Cap /proc/$$/status
> >    CapInh:     0000000000000000
> >    CapPrm:     fffffff800001000
> >    CapEff:     fffffff800001000
> >    CapBnd:     fffffff800001000
> >    root@albatros:~# modprobe xfs
> >    FATAL: Error inserting xfs
> >    (/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted
> >    root@albatros:~# lsmod | grep xfs
> >    root@albatros:~# ifconfig xfs
> >    xfs: error fetching interface information: Device not found
> >    root@albatros:~# lsmod | grep xfs
> >    root@albatros:~# lsmod | grep sit
> >    root@albatros:~# ifconfig sit
> >    sit: error fetching interface information: Device not found
> >    root@albatros:~# lsmod | grep sit
> >    root@albatros:~# ifconfig sit0
> >    sit0      Link encap:IPv6-in-IPv4
> >              NOARP  MTU:1480  Metric:1
> >
> >    root@albatros:~# lsmod | grep sit
> >    sit                    10457  0
> >    tunnel4                 2957  1 sit
> >
> > For CAP_SYS_MODULE module loading is still relaxed:
> >
> >    root@albatros:~# grep Cap /proc/$$/status
> >    CapInh:     0000000000000000
> >    CapPrm:     ffffffffffffffff
> >    CapEff:     ffffffffffffffff
> >    CapBnd:     ffffffffffffffff
> >    root@albatros:~# ifconfig xfs
> >    xfs: error fetching interface information: Device not found
> >    root@albatros:~# lsmod | grep xfs
> >    xfs                   745319  0
> >
> > Reference: https://lkml.org/lkml/2011/2/24/203
> >
> > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> > ---
> >  v2 - use pr_err()
> >    - don't try to load $name if netdev-$name is loaded
> >
> >  include/linux/netdevice.h |    3 +++
> >  net/core/dev.c            |   12 ++++++++++--
> >  net/ipv4/ip_gre.c         |    2 +-
> >  net/ipv4/ipip.c           |    2 +-
> >  net/ipv6/sit.c            |    2 +-
> >  5 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index d971346..71caf7a 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...)
> >  extern int netdev_info(const struct net_device *dev, const char *format, ...)
> >        __attribute__ ((format (printf, 2, 3)));
> >
> > +#define MODULE_ALIAS_NETDEV(device) \
> > +       MODULE_ALIAS("netdev-" device)
> > +
> >  #if defined(DEBUG)
> >  #define netdev_dbg(__dev, format, args...)                     \
> >        netdev_printk(KERN_DEBUG, __dev, format, ##args)
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 8ae6631..6561021 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1114,13 +1114,21 @@ EXPORT_SYMBOL(netdev_bonding_change);
> >  void dev_load(struct net *net, const char *name)
> >  {
> >        struct net_device *dev;
> > +       int no_module;
> >
> >        rcu_read_lock();
> >        dev = dev_get_by_name_rcu(net, name);
> >        rcu_read_unlock();
> >
> > -       if (!dev && capable(CAP_NET_ADMIN))
> > -               request_module("%s", name);
> > +       no_module = !dev;
> > +       if (no_module && capable(CAP_NET_ADMIN))
> > +               no_module = request_module("netdev-%s", name);
> > +       if (no_module && capable(CAP_SYS_MODULE)) {
> > +               if (!request_module("%s", name))
> > +                       pr_err("Loading kernel module for a network device "
> > +"with CAP_SYS_MODULE (deprecated).  Use CAP_NET_ADMIN and alias netdev-%s "
> > +"instead\n", name);
> > +       }
> >  }
> >  EXPORT_SYMBOL(dev_load);
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index 6613edf..d1d0e2c 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -1765,4 +1765,4 @@ module_exit(ipgre_fini);
> >  MODULE_LICENSE("GPL");
> >  MODULE_ALIAS_RTNL_LINK("gre");
> >  MODULE_ALIAS_RTNL_LINK("gretap");
> > -MODULE_ALIAS("gre0");
> > +MODULE_ALIAS_NETDEV("gre0");
> > diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
> > index 988f52f..a5f58e7 100644
> > --- a/net/ipv4/ipip.c
> > +++ b/net/ipv4/ipip.c
> > @@ -913,4 +913,4 @@ static void __exit ipip_fini(void)
> >  module_init(ipip_init);
> >  module_exit(ipip_fini);
> >  MODULE_LICENSE("GPL");
> > -MODULE_ALIAS("tunl0");
> > +MODULE_ALIAS_NETDEV("tunl0");
> > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> > index 8ce38f1..d2c16e1 100644
> > --- a/net/ipv6/sit.c
> > +++ b/net/ipv6/sit.c
> > @@ -1290,4 +1290,4 @@ static int __init sit_init(void)
> >  module_init(sit_init);
> >  module_exit(sit_cleanup);
> >  MODULE_LICENSE("GPL");
> > -MODULE_ALIAS("sit0");
> > +MODULE_ALIAS_NETDEV("sit0");
> > --
> > 1.7.0.4
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

  reply	other threads:[~2011-03-24 15:38 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-24 15:12 module loading with CAP_NET_ADMIN Vasiliy Kulikov
2011-02-24 16:34 ` Ben Hutchings
2011-02-25 12:30   ` Vasiliy Kulikov
2011-02-25 15:14     ` [PATCH] don't allow CAP_NET_ADMIN to load non-netdev kernel modules Vasiliy Kulikov
2011-02-25 17:25       ` Valdis.Kletnieks
2011-02-25 17:47         ` Vasiliy Kulikov
2011-02-25 17:48         ` Ben Hutchings
2011-02-25 18:47       ` David Miller
2011-02-25 19:02         ` Vasiliy Kulikov
2011-02-25 19:05           ` David Miller
2011-02-25 19:07             ` Ben Hutchings
2011-02-25 19:16               ` David Miller
2011-02-25 19:30                 ` Ben Hutchings
2011-02-25 19:43                   ` David Miller
2011-02-25 19:53                     ` Ben Hutchings
2011-02-25 20:37                       ` David Miller
2011-02-25 20:38                       ` Ben Hutchings
2011-02-25 20:59                         ` Michał Mirosław
2011-02-27 20:22                           ` Arnd Bergmann
2011-02-28  9:29                             ` Michael Tokarev
2011-02-28  9:51                               ` Vasiliy Kulikov
2011-02-28 19:23                                 ` David Miller
2011-03-01 19:48                                   ` [PATCH] net: " Vasiliy Kulikov
2011-03-01 20:13                                     ` Ben Hutchings
2011-03-01 21:33                                       ` [PATCH v2] " Vasiliy Kulikov
2011-03-02  7:15                                         ` Michael Tokarev
2011-03-09 22:06                                           ` Vasiliy Kulikov
2011-03-09 22:09                                             ` David Miller
2011-03-09 22:53                                               ` James Morris
2011-03-10  9:49                                                 ` Vasiliy Kulikov
2011-03-02 16:01                                         ` Kees Cook
2011-03-02 19:39                                         ` Jake Edge
2011-03-02 19:43                                           ` Vasiliy Kulikov
2011-03-02 19:49                                             ` Jake Edge
2011-03-02 20:18                                               ` Vasiliy Kulikov
2011-03-02 20:38                                                 ` Jake Edge
2011-03-02 20:40                                                 ` Jake Edge
2011-03-22 20:47                                         ` Eric Paris
2011-03-24 15:37                                           ` Serge E. Hallyn [this message]
2011-03-24 18:03                                             ` Eric Paris
2011-03-24 18:33                                               ` Ben Hutchings
2011-03-24 20:26                                                 ` Serge E. Hallyn
2011-03-24 21:39                                                   ` Stephen Hemminger
2011-03-24 21:46                                                     ` David Miller
2011-03-24 21:57                                                       ` Serge E. Hallyn
2011-03-24 22:15                                                         ` Eric Paris
2011-03-24 21:57                                                       ` Greg KH
2011-03-26 10:35                                                       ` Vasiliy Kulikov
2011-02-27 11:44                 ` [PATCH] " Vasiliy Kulikov
2011-02-27 23:18                   ` David Miller
2011-02-27 23:19                   ` David Miller
2011-02-25 15:29     ` module loading with CAP_NET_ADMIN Michael Tokarev
2011-02-25 15:57       ` Vasiliy Kulikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110324153714.GB2648@peq.hallyn.com \
    --to=serge.hallyn@ubuntu.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bhutchings@solarflare.com \
    --cc=dan.j.rosenberg@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dwalsh@redhat.com \
    --cc=eparis@parisplace.org \
    --cc=eric.dumazet@gmail.com \
    --cc=eugene@redhat.com \
    --cc=greg@kroah.com \
    --cc=jesse@nicira.com \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=kees.cook@canonical.com \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mirqus@gmail.com \
    --cc=mjt@tls.msk.ru \
    --cc=netdev@vger.kernel.org \
    --cc=pekkas@netcore.fi \
    --cc=sds@tycho.nsa.gov \
    --cc=segoon@openwall.com \
    --cc=therbert@google.com \
    --cc=xiaosuo@gmail.com \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®