Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932857AbbGJQ5G (ORCPT ); Fri, 10 Jul 2015 12:57:06 -0400 Received: from emvm-gh1-uea08.nsa.gov ([63.239.67.9]:54592 "EHLO emvm-gh1-uea08.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932290AbbGJQ4z (ORCPT ); Fri, 10 Jul 2015 12:56:55 -0400 X-TM-IMSS-Message-ID: <8a498230001154b7@nsa.gov> Message-ID: <559FF920.2020302@tycho.nsa.gov> Date: Fri, 10 Jul 2015 12:56:00 -0400 From: Stephen Smalley Organization: National Security Agency User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Paul Osmialowski , Paul Moore , James Morris , Casey Schaufler , "Serge E. Hallyn" , Kees Cook , Tetsuo Handa , Neil Brown , Mark Rustad , Greg Kroah-Hartman , Daniel Mack , David Herrmann , Djalal Harouni , Shuah Khan , Al Viro , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, linux-api@vger.kernel.org CC: Karol Lewandowski , Lukasz Skalski Subject: Re: [RFC 5/8] kdbus: use LSM hooks in kdbus code References: <1436351110-5902-1-git-send-email-p.osmialowsk@samsung.com> <1436351110-5902-6-git-send-email-p.osmialowsk@samsung.com> <559D27AB.4010402@tycho.nsa.gov> In-Reply-To: <559D27AB.4010402@tycho.nsa.gov> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3201 Lines: 89 On 07/08/2015 09:37 AM, Stephen Smalley wrote: > On 07/08/2015 06:25 AM, Paul Osmialowski wrote: >> Originates from: >> >> https://github.com/lmctl/kdbus.git (branch: kdbus-lsm-v4.for-systemd-v212) >> commit: aa0885489d19be92fa41c6f0a71df28763228a40 >> >> Signed-off-by: Karol Lewandowski >> Signed-off-by: Paul Osmialowski >> --- >> ipc/kdbus/bus.c | 12 ++++++++++- >> ipc/kdbus/bus.h | 3 +++ >> ipc/kdbus/connection.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> ipc/kdbus/connection.h | 4 ++++ >> ipc/kdbus/domain.c | 9 ++++++++- >> ipc/kdbus/domain.h | 2 ++ >> ipc/kdbus/endpoint.c | 11 ++++++++++ >> ipc/kdbus/names.c | 11 ++++++++++ >> ipc/kdbus/queue.c | 30 ++++++++++++++++++---------- >> 9 files changed, 124 insertions(+), 12 deletions(-) >> >> > >> diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c >> index 9993753..b85cdc7 100644 >> --- a/ipc/kdbus/connection.c >> +++ b/ipc/kdbus/connection.c >> @@ -31,6 +31,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "bus.h" >> #include "connection.h" >> @@ -73,6 +74,8 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged, >> bool is_activator; >> bool is_monitor; >> struct kvec kvec; >> + u32 sid, len; >> + char *label; >> int ret; >> >> struct { >> @@ -222,6 +225,14 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged, >> } >> } >> >> + security_task_getsecid(current, &sid); >> + security_secid_to_secctx(sid, &label, &len); >> + ret = security_kdbus_connect(conn, label, len); >> + if (ret) { >> + ret = -EPERM; >> + goto exit_unref; >> + } > > This seems convoluted and expensive. If you always want the label of > the current task here, then why not just have security_kdbus_connect() > internally extract the label of the current task? > >> @@ -1107,6 +1119,12 @@ static int kdbus_conn_reply(struct kdbus_conn *src, struct kdbus_kmsg *kmsg) >> if (ret < 0) >> goto exit; >> >> + ret = security_kdbus_talk(src, dst); >> + if (ret) { >> + ret = -EPERM; >> + goto exit; >> + } > > Where does kdbus apply its uid-based or other restrictions on > connections? Why do we need to insert separate hooks into each of these > functions? Is there no central chokepoint already for permission > checking that we can hook? For example, why wouldn't you insert a single hook into kdbus_conn_policy_talk() where they perform their DAC checking? You would need to restructure it slightly to ensure that the security hook is only called if it passes the DAC (privileged || uid_eq) check so that we do not trigger MAC denials when DAC wouldn't have allowed it anyway. Also, kdbus_conn_policy_talk() takes a separate conn_creds argument - that should be passed through to the hook as well. -- 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/