netdev - Re: [PATCH 5/5] net: Use netlink_ns_capable to verify the permisions of netlink messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 7 May 2014 15:26:11 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	"Jorge Boncompte [DTI2]" <jorge@...2.net>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	David Miller <davem@...emloft.net>,
	Vivek Goyal <vgoyal@...hat.com>,
	Simo Sorce <ssorce@...hat.com>,
	"security@...nel.org" <security@...nel.org>,
	Network Development <netdev@...r.kernel.org>,
	"Serge E. Hallyn" <serge@...lyn.com>
Subject: Re: [PATCH 5/5] net: Use netlink_ns_capable to verify the permisions
 of netlink messages

On Wed, May 7, 2014 at 3:18 PM, Jorge Boncompte [DTI2] <jorge@...2.net> wrote:
> El 23/04/2014 23:29, Eric W. Biederman escribió:
>>
>> It is possible by passing a netlink socket to a more privileged
>> executable and then to fool that executable into writing to the
>> socket data that happens to be valid netlink message to do something
>> that privileged executable did not intend to do.
>>
>> To keep this from happening replace bare capable and ns_capable calls
>> with netlink_capable, netlink_net_calls and netlink_ns_capable calls.
>> Which act the same as the previous calls except they verify that the
>> opener of the socket had the desired permissions as well.
>>
>
>         Hi, after this patch, zebra daemon of quagga in Debian testing fails to
> send routes to kernel with an -EPERM error.
>         Reverting this patch and commit a64d90fd96 (netfilter: Fix warning in
> nfnetlink_receive().) fixes it for me.
>
>         I haven't got time to do a proper analisys and could be that zebra
> it's doing something silly but this patch seems to subtly change some
> semantics.
>

Well, crap.

  sock = socket (AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
  if (sock < 0)
    {
      zlog (NULL, LOG_ERR, "Can't open %s socket: %s", nl->name,
            safe_strerror (errno));
      return -1;
    }

  memset (&snl, 0, sizeof snl);
  snl.nl_family = AF_NETLINK;
  snl.nl_groups = groups;

  /* Bind the socket to the netlink structure for anything. */
  if (zserv_privs.change (ZPRIVS_RAISE))
    {
      zlog (NULL, LOG_ERR, "Can't raise privileges");
      return -1;
    }

  ret = bind (sock, (struct sockaddr *) &snl, sizeof snl);
  save_errno = errno;
  if (zserv_privs.change (ZPRIVS_LOWER))
    zlog (NULL, LOG_ERR, "Can't lower privileges");

  if (ret < 0)
    {
      zlog (NULL, LOG_ERR, "Can't bind %s socket to group 0x%x: %s",
            nl->name, snl.nl_groups, safe_strerror (save_errno));
      close (sock);
      return -1;
    }

So what do we do?  Check permissions on connect and then use the
cached result for send on a connected socket?  Check permitted caps
instead of effective caps?

This sucks.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html