From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932517AbeDWRwk (ORCPT ); Mon, 23 Apr 2018 13:52:40 -0400 Received: from h2.hallyn.com ([78.46.35.8]:58726 "EHLO mail.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932204AbeDWRwf (ORCPT ); Mon, 23 Apr 2018 13:52:35 -0400 Date: Mon, 23 Apr 2018 12:52:33 -0500 From: "Serge E. Hallyn" To: David Herrmann Cc: linux-kernel@vger.kernel.org, James Morris , Paul Moore , teg@jklm.no, Stephen Smalley , selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org, Eric Paris , serge@hallyn.com, davem@davemloft.net, netdev@vger.kernel.org Subject: Re: [PATCH 0/3] Introduce LSM-hook for socketpair(2) Message-ID: <20180423175233.GA4284@mail.hallyn.com> References: <20180423133015.5455-1-dh.herrmann@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180423133015.5455-1-dh.herrmann@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting David Herrmann (dh.herrmann@gmail.com): > Hi > > This series adds a new LSM hook for the socketpair(2) syscall. The idea > is to allow SO_PEERSEC to be called on AF_UNIX sockets created via > socketpair(2), and return the same information as if you emulated > socketpair(2) via a temporary listener socket. Right now SO_PEERSEC > will return the unlabeled credentials for a socketpair, rather than the > actual credentials of the creating process. > > A simple call to: > > socketpair(AF_UNIX, SOCK_STREAM, 0, out); > > can be emulated via a temporary listener socket bound to a unique, > random name in the abstract namespace. By connecting to this listener > socket, accept(2) will return the second part of the pair. If > SO_PEERSEC is queried on these, the correct credentials of the creating > process are returned. A simple comparison between the behavior of > SO_PEERSEC on socketpair(2) and an emulated socketpair is included in > the dbus-broker test-suite [1]. > > This patch series tries to close this gap and makes both behave the > same. A new LSM-hook is added which allows LSMs to cache the correct > peer information on newly created socket-pairs. > > Apart from fixing this behavioral difference, the dbus-broker project > actually needs to query the credentials of socketpairs, and currently > must resort to racy procfs(2) queries to get the LSM credentials of its > controller socket. Several parts of the dbus-broker project allow you > to pass in a socket during execve(2), which will be used by the child > process to accept control-commands from its parent. One natural way to > create this communication channel is to use socketpair(2). However, > right now SO_PEERSEC does not return any useful information, hence, the > child-process would need other means to retrieve this information. By > avoiding socketpair(2) and using the hacky-emulated version, this is not > an issue. > > There was a previous discussion on this matter [2] roughly a year ago. > Back then there was the suspicion that proper SO_PEERSEC would confuse > applications. However, we could not find any evidence backing this > suspicion. Furthermore, we now actually see the contrary. Lack of > SO_PEERSEC makes it a hassle to use socketpairs with LSM credentials. > Hence, we propose to implement full SO_PEERSEC for socketpairs. > > This series only adds SELinux backends, since that is what we need for > RHEL. I will gladly extend the other LSMs if needed. > > Thanks > David > > [1] https://github.com/bus1/dbus-broker/blob/master/src/util/test-peersec.c > [2] https://www.spinics.net/lists/selinux/msg22674.html > > David Herrmann (3): > security: add hook for socketpair(AF_UNIX, ...) > net/unix: hook unix_socketpair() into LSM > selinux: provide unix_stream_socketpair callback > > include/linux/lsm_hooks.h | 8 ++++++++ > include/linux/security.h | 7 +++++++ > net/unix/af_unix.c | 5 +++++ > security/security.c | 6 ++++++ > security/selinux/hooks.c | 14 ++++++++++++++ > 5 files changed, 40 insertions(+) Makes sense to me, thanks. Acked-by: Serge Hallyn