From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-2520872-1519769777-2-17061506571512814894 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, HEADER_FROM_DIFFERENT_DOMAINS 0.249, ME_NOAUTH 0.01, RCVD_IN_DNSWL_HI -5, T_RP_MATCHES_RCVD -0.01, T_TVD_MIME_EPI 0.01, LANGUAGES en, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='209.132.180.67', Host='vger.kernel.org', Country='CN', FromHeader='net', MailFrom='org' X-Spam-charsets: from='UTF-8', plain='utf-8' X-Attached: signature.asc X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: linux-api-owner@vger.kernel.org ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1519769776; b=iudnvHVLIC7/ZO83scaTH8ZrUUcToxd1QhC/nwXWgEmDwA1 xvKYBY3sXjczsXXb0hgmmhcI/fE76Ph0c60gYvgVCjV0yGmIVazQlnyzPCif3g7f 6NCQYqk63dfhjcVEKHOwUXxU2iOizXWVLyJEtzmYi4Ev8oiD0uhSAQBPvfe4yRh4 Csubcb64E5gBHpzweJM6GIpHoT3p3lk6CAUo05g1fk5ToIrpSUSpwpvW32H+6gQc U9g7t99XG1nPrINCT2ZE/ntxcGAMKEEZba4QqRr/v3AV2BMlgocBcyJx4LUKyMa9 qWt8FnjGHogT78LXAQWlbDL++FBcIteEi/hrgIA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=subject:to:cc:references:from:message-id :date:mime-version:in-reply-to:content-type:sender:list-id; s= arctest; t=1519769776; bh=h7sRJISBiT+dEYbXSrGQiAx17hIekvhM5ua0bT Ow6Ks=; b=hPzfpBblM7qzbSjO0Nuv4zfDgDLVkzxd/EPm0TDSrXEaHNC8sfzy9d OA0jghmJIy04sJaeyA/FVVI5UQq61b+DLWP2ubyQTp/l7aQYteQpsBYMs33/IBMw hJxTvoYipnDLvZ/5gZcMGWUlcWsgJx0KWNeXOwStSQwXg+qqjX/zbr5jaLgOKd1u Plq92CADnd4XHt05lrNDz658VP0SEmgA8mR7d662J0UB1uDk+pS7pkvOCkfayw6r QIhFO1BvzCB+PFg8dEQBHd3210jYw7He0mlsIHhHaj+S86syG/ZN83AEo48qsUyw ZkN8qBuRDekA5DsIvcJ8FJkImY+b/wfA== ARC-Authentication-Results: i=1; mx6.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,has-list-id=yes,d=none) header.from=digikod.net; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=linux-api-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=digikod.net header.result=pass header_is_org_domain=yes Authentication-Results: mx6.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,has-list-id=yes,d=none) header.from=digikod.net; iprev=pass policy.iprev=209.132.180.67 (vger.kernel.org); spf=none smtp.mailfrom=linux-api-owner@vger.kernel.org smtp.helo=vger.kernel.org; x-aligned-from=fail; x-ptr=pass x-ptr-helo=vger.kernel.org x-ptr-lookup=vger.kernel.org; x-return-mx=pass smtp.domain=vger.kernel.org smtp.result=pass smtp_org.domain=kernel.org smtp_org.result=pass smtp_is_org_domain=no header.domain=digikod.net header.result=pass header_is_org_domain=yes Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751735AbeB0WQO (ORCPT ); Tue, 27 Feb 2018 17:16:14 -0500 Received: from smtp-sh.infomaniak.ch ([128.65.195.4]:42732 "EHLO smtp-sh.infomaniak.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751552AbeB0WQM (ORCPT ); Tue, 27 Feb 2018 17:16:12 -0500 Subject: Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions To: Andy Lutomirski Cc: LKML , Alexei Starovoitov , Arnaldo Carvalho de Melo , Casey Schaufler , Daniel Borkmann , David Drysdale , "David S . Miller" , "Eric W . Biederman" , James Morris , Jann Horn , Jonathan Corbet , Michael Kerrisk , Kees Cook , Paul Moore , Sargun Dhillon , "Serge E . Hallyn" , Shuah Khan , Tejun Heo , Thomas Graf , Tycho Andersen , Will Drewry , Kernel Hardening , Linux API , LSM List , Network Development References: <20180227004121.3633-1-mic@digikod.net> <20180227004121.3633-9-mic@digikod.net> From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= Message-ID: <0e7d0512-12a3-568d-aa55-3def4b91c6d0@digikod.net> Date: Tue, 27 Feb 2018 23:14:58 +0100 User-Agent: MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="juzjA3tOq9X7GzHiBhdEjTC1LSPJGxe5L" X-Antivirus-Code: 0x100000 Sender: linux-api-owner@vger.kernel.org X-Mailing-List: linux-api@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --juzjA3tOq9X7GzHiBhdEjTC1LSPJGxe5L Content-Type: multipart/mixed; boundary="mHkglwZfixZ6jp0eTeXn92TrtSd9RNwPP"; protected-headers="v1" From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= To: Andy Lutomirski Cc: LKML , Alexei Starovoitov , Arnaldo Carvalho de Melo , Casey Schaufler , Daniel Borkmann , David Drysdale , "David S . Miller" , "Eric W . Biederman" , James Morris , Jann Horn , Jonathan Corbet , Michael Kerrisk , Kees Cook , Paul Moore , Sargun Dhillon , "Serge E . Hallyn" , Shuah Khan , Tejun Heo , Thomas Graf , Tycho Andersen , Will Drewry , Kernel Hardening , Linux API , LSM List , Network Development Message-ID: <0e7d0512-12a3-568d-aa55-3def4b91c6d0@digikod.net> Subject: Re: [PATCH bpf-next v8 08/11] landlock: Add ptrace restrictions References: <20180227004121.3633-1-mic@digikod.net> <20180227004121.3633-9-mic@digikod.net> In-Reply-To: --mHkglwZfixZ6jp0eTeXn92TrtSd9RNwPP Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 27/02/2018 06:01, Andy Lutomirski wrote: >=20 >=20 >> On Feb 26, 2018, at 8:17 PM, Andy Lutomirski wro= te: >> >>> On Tue, Feb 27, 2018 at 12:41 AM, Micka=C3=ABl Sala=C3=BCn wrote: >>> A landlocked process has less privileges than a non-landlocked proces= s >>> and must then be subject to additional restrictions when manipulating= >>> processes. To be allowed to use ptrace(2) and related syscalls on a >>> target process, a landlocked process must have a subset of the target= >>> process' rules. >>> >>> Signed-off-by: Micka=C3=ABl Sala=C3=BCn >>> Cc: Alexei Starovoitov >>> Cc: Andy Lutomirski >>> Cc: Daniel Borkmann >>> Cc: David S. Miller >>> Cc: James Morris >>> Cc: Kees Cook >>> Cc: Serge E. Hallyn >>> --- >>> >>> Changes since v6: >>> * factor out ptrace check >>> * constify pointers >>> * cleanup headers >>> * use the new security_add_hooks() >>> --- >>> security/landlock/Makefile | 2 +- >>> security/landlock/hooks_ptrace.c | 124 ++++++++++++++++++++++++++++++= +++++++++ >>> security/landlock/hooks_ptrace.h | 11 ++++ >>> security/landlock/init.c | 2 + >>> 4 files changed, 138 insertions(+), 1 deletion(-) >>> create mode 100644 security/landlock/hooks_ptrace.c >>> create mode 100644 security/landlock/hooks_ptrace.h >>> >>> diff --git a/security/landlock/Makefile b/security/landlock/Makefile >>> index d0f532a93b4e..605504d852d3 100644 >>> --- a/security/landlock/Makefile >>> +++ b/security/landlock/Makefile >>> @@ -3,4 +3,4 @@ obj-$(CONFIG_SECURITY_LANDLOCK) :=3D landlock.o >>> landlock-y :=3D init.o chain.o task.o \ >>> tag.o tag_fs.o \ >>> enforce.o enforce_seccomp.o \ >>> - hooks.o hooks_cred.o hooks_fs.o >>> + hooks.o hooks_cred.o hooks_fs.o hooks_ptrace.o >>> diff --git a/security/landlock/hooks_ptrace.c b/security/landlock/hoo= ks_ptrace.c >>> new file mode 100644 >>> index 000000000000..f1b977b9c808 >>> --- /dev/null >>> +++ b/security/landlock/hooks_ptrace.c >>> @@ -0,0 +1,124 @@ >>> +/* >>> + * Landlock LSM - ptrace hooks >>> + * >>> + * Copyright =C2=A9 2017 Micka=C3=ABl Sala=C3=BCn >>> + * >>> + * This program is free software; you can redistribute it and/or mod= ify >>> + * it under the terms of the GNU General Public License version 2, a= s >>> + * published by the Free Software Foundation. >>> + */ >>> + >>> +#include >>> +#include >>> +#include /* ARRAY_SIZE */ >>> +#include >>> +#include /* struct task_struct */ >>> +#include >>> + >>> +#include "common.h" /* struct landlock_prog_set */ >>> +#include "hooks.h" /* landlocked() */ >>> +#include "hooks_ptrace.h" >>> + >>> +static bool progs_are_subset(const struct landlock_prog_set *parent,= >>> + const struct landlock_prog_set *child) >>> +{ >>> + size_t i; >>> + >>> + if (!parent || !child) >>> + return false; >>> + if (parent =3D=3D child) >>> + return true; >>> + >>> + for (i =3D 0; i < ARRAY_SIZE(child->programs); i++) { >> >> ARRAY_SIZE(child->programs) seems misleading. Is there no define >> NUM_LANDLOCK_PROG_TYPES or similar? >> >>> + struct landlock_prog_list *walker; >>> + bool found_parent =3D false; >>> + >>> + if (!parent->programs[i]) >>> + continue; >>> + for (walker =3D child->programs[i]; walker; >>> + walker =3D walker->prev) { >>> + if (walker =3D=3D parent->programs[i]) { >>> + found_parent =3D true; >>> + break; >>> + } >>> + } >>> + if (!found_parent) >>> + return false; >>> + } >>> + return true; >>> +} >> >> If you used seccomp, you'd get this type of check for free, and it >> would be a lot easier to comprehend. AFAICT the only extra leniency >> you're granting is that you're agnostic to the order in which the >> rules associated with different program types were applied, which >> could easily be added to seccomp. >=20 > On second thought, this is all way too complicated. I think the correc= t logic is either "if you are filtered by Landlock, you cannot ptrace any= thing" or to delete this patch entirely. This does not fit a lot of use cases like running a container constrained with some Landlock programs. We should not deny users the ability to debug their stuff. This patch add the minimal protection which are needed to have meaningful Landlock security policy. Without it, they may be easily bypassable, hence useless. > If something like Tycho's notifiers goes in, then it's not obvious that= , just because you have the same set of filters, you have the same privil= ege. Similarly, if a feature that lets a filter query its cgroup goes in= (and you proposed this once!) then the logic you implemented here is wro= ng. I don't get your point. Please take a look at the tests (patch 10). >=20 > Or you could just say that it's the responsibility of a Landlock user t= o properly filter ptrace() just like it's the responsibility of seccomp u= sers to filter ptrace if needed. A user should be able to enforce a security policy on ptrace as well, but this patch enforce a minimal set of security boundaries. It will be easy to add a new Landlock program type to get this kind of access contro= l. >=20 > I take this as further evidence that Landlock makes much more sense as = part of seccomp than as a totally separate thing. We've very carefully r= eviewed these things for seccomp. Please don't make us do it again from = scratch. >=20 Landlock is more complex than seccomp, because of its different goal. seccomp is less restrictive because it is more simple, but this patch follow the same logic with the knowledge of the Landlock internals. --mHkglwZfixZ6jp0eTeXn92TrtSd9RNwPP-- --juzjA3tOq9X7GzHiBhdEjTC1LSPJGxe5L Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEUysCyY8er9Axt7hqIt7+33O9apUFAlqV2GIACgkQIt7+33O9 apXEyQf+KURddBvKxYNuwM6u0k7oFAC+stSeLTkBHdfBXv7UEvAgfPpBAVmyqYv4 mx2gBn85Ey1redPA1W9ezeX6AzuDTcu/mDlpp/h7+CeTG2eSGp4XGVxHCBbuQLKT CsWq4syeo1cHy11YKv/GPDDo1ZxwJlr7ICu5WYBQMy37fj3kqE5iHcmW7InBYKWa +POKR3oY+nU9iNxO1qj/LgdXC3dSR6xJZfK0q27uAE293cEzeHZ2+yx8zOGRCVkq dhKz87tLb3RI0Wwt8dT0ihQNBwWG63bD+rZ4KCjUiZI/QeBps5LpLsRa5tQMKOKN PoF3ZlCcpZkDhVYdyKuumxBEnrtxFA== =9JFP -----END PGP SIGNATURE----- --juzjA3tOq9X7GzHiBhdEjTC1LSPJGxe5L--