Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp542847pxj; Wed, 2 Jun 2021 05:44:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzKPgjbSCn0uybMQ/p2C89Lfg43MTh98gXKuX7SyuFc425ixbluxiYhlFxitLoVrZPct8Tf X-Received: by 2002:a05:6402:3101:: with SMTP id dc1mr23398765edb.324.1622637850299; Wed, 02 Jun 2021 05:44:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1622637850; cv=none; d=google.com; s=arc-20160816; b=ktWTXoU6N4IPhW4LGvyrkq4WL21sohuQclxy2yq+xfgJycWm6NCoJqTnt/gh953gQG b42OqvI2tSLghZGjVsZWJiaAaD2Nzr3CYR93dLzt5MmtfM6o9v77j1OZ96+j9xtAGktQ x3GAyLLMd1R1O/SDeH9Ttu9jTB2V9ZBEkLfqc9k7Sz+mwGNvX8+/V8Z8mhk7GNWS6ZJC d1AZpuZiTIF3aUNzyc2WxK2/djzmTg/A/fsDzIdqrc6FHR2gHLBmM6ArcvTQEJ2sqJCc e/2GnZukZAnaV6Waok9lqdLUHQhtt1ImT6nGBuntXf67tRitFNGpOlYVHNI4L0qB7sDf Kr4A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject; bh=jGAC6dlcF0T17kmmhZWiwGTIcwRjWIZfo8n7evKJklk=; b=yNsCL9C5Hme0/trNZYGpebzIZc8BWCT/lCn82AD+HB51W5UVTEFWmYuPFwhzO6spHD SNDTPfzgfvQ9txYXV+7xoryKsU8Gro7u6dMcH+wpPJjl/HHfnOkmQD2u3vot5qlcDgO+ nZu8oUqmu1g1efQbAqBAoGJMaIEoNhm5e2GZDfGkxFA+g3M72ePY+cFvLjvmgdBMnO/Q WewIKT96zWQeVNwAf0RAk6bCDPZb/xz2IIMsnFA4qcer8D3tJ0h6qX7NNNY8nN6y4mAd PaQ8Ib2ftttKdYWZWeJjDnzTOLodLqnuGpH6oS/kgDrTtQ6xWhcPYBiYgJ+M8GZFUkUC POuA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id df14si1837666edb.108.2021.06.02.05.43.47; Wed, 02 Jun 2021 05:44:10 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230008AbhFBMmX (ORCPT + 99 others); Wed, 2 Jun 2021 08:42:23 -0400 Received: from www62.your-server.de ([213.133.104.62]:46128 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229656AbhFBMmV (ORCPT ); Wed, 2 Jun 2021 08:42:21 -0400 Received: from sslproxy05.your-server.de ([78.46.172.2]) by www62.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1loQAj-000D4R-Ns; Wed, 02 Jun 2021 14:40:33 +0200 Received: from [2001:1620:665:0:5795:5b0a:e5d5:5944] (helo=linux-3.fritz.box) by sslproxy05.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1loQAj-000MyV-DJ; Wed, 02 Jun 2021 14:40:33 +0200 Subject: Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks To: Paul Moore Cc: Ondrej Mosnacek , linux-security-module@vger.kernel.org, James Morris , Steven Rostedt , Ingo Molnar , Stephen Smalley , selinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, bpf@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Casey Schaufler , jolsa@redhat.com References: <20210517092006.803332-1-omosnace@redhat.com> <01135120-8bf7-df2e-cff0-1d73f1f841c3@iogearbox.net> <2e541bdc-ae21-9a07-7ac7-6c6a4dda09e8@iogearbox.net> From: Daniel Borkmann Message-ID: <3ca181e3-df32-9ae0-12c6-efb899b7ce7a@iogearbox.net> Date: Wed, 2 Jun 2021 14:40:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.2/26189/Wed Jun 2 13:10:34 2021) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6/1/21 10:47 PM, Paul Moore wrote: > On Mon, May 31, 2021 at 4:24 AM Daniel Borkmann wrote: >> On 5/29/21 8:48 PM, Paul Moore wrote: >> [...] >>> Daniel's patch side steps that worry by just doing the lockdown >>> permission check when the BPF program is loaded, but that isn't a >>> great solution if the policy changes afterward. I was hoping there >>> might be some way to perform the permission check as needed, but the >>> more I look the more that appears to be difficult, if not impossible >>> (once again, corrections are welcome). >> >> Your observation is correct, will try to clarify below a bit. >> >>> I'm now wondering if the right solution here is to make use of the LSM >>> notifier mechanism. I'm not yet entirely sure if this would work from >>> a BPF perspective, but I could envision the BPF subsystem registering >>> a LSM notification callback via register_blocking_lsm_notifier(), see >>> if Infiniband code as an example, and then when the LSM(s) policy >>> changes the BPF subsystem would get a notification and it could >>> revalidate the existing BPF programs and take block/remove/whatever >>> the offending BPF programs. This obviously requires a few things >>> which I'm not sure are easily done, or even possible: >>> >>> 1. Somehow the BPF programs would need to be "marked" at >>> load/verification time with respect to their lockdown requirements so >>> that decisions can be made later. Perhaps a flag in bpf_prog_aux? >>> >>> 2. While it looks like it should be possible to iterate over all of >>> the loaded BPF programs in the LSM notifier callback via >>> idr_for_each(prog_idr, ...), it is not clear to me if it is possible >>> to safely remove, or somehow disable, BPF programs once they have been >>> loaded. Hopefully the BPF folks can help answer that question. >>> >>> 3. Disabling of BPF programs might be preferable to removing them >>> entirely on LSM policy changes as it would be possible to make the >>> lockdown state less restrictive at a future point in time, allowing >>> for the BPF program to be executed again. Once again, not sure if >>> this is even possible. >> >> Part of why this gets really complex/impossible is that BPF programs in >> the kernel are reference counted from various sides, be it that there >> are references from user space to them (fd from application, BPF fs, or >> BPF links), hooks where they are attached to as well as tail call maps >> where one BPF prog calls into another. There is currently also no global >> infra of some sort where you could piggy back to atomically keep track of >> all the references in a list or such. And the other thing is that BPF progs >> have no ownership that is tied to a specific task after they have been >> loaded. Meaning, once they are loaded into the kernel by an application >> and attached to a specific hook, they can remain there potentially until >> reboot of the node, so lifecycle of the user space application != lifecycle >> of the BPF program. > > I don't think the disjoint lifecycle or lack of task ownership is a > deal breaker from a LSM perspective as the LSMs can stash whatever > info they need in the security pointer during the program allocation > hook, e.g. selinux_bpf_prog_alloc() saves the security domain which > allocates/loads the BPF program. > > The thing I'm worried about would be the case where a LSM policy > change requires that an existing BPF program be removed or disabled. > I'm guessing based on the refcounting that there is not presently a > clean way to remove a BPF program from the system, but is this > something we could resolve? If we can't safely remove a BPF program > from the system, can we replace/swap it with an empty/NULL BPF > program? Removing progs would somehow mean destroying those references from an async event and then /safely/ guaranteeing that nothing is accessing them anymore. But then if policy changes once more where they would be allowed again we would need to revert back to the original state, which brings us to your replace/swap question with an empty/null prog. It's not feasible either, because there are different BPF program types and they can have different return code semantics that lead to subsequent actions. If we were to replace them with an empty/NULL program, then essentially this will get us into an undefined system state given it's unclear what should be a default policy for each program type, etc. Just to pick one simple example, outside of tracing, that comes to mind: say, you attached a program with tc to a given device ingress hook. That program implements firewalling functionality, and potentially deep down in that program there is functionality to record/sample packets along with some meta data. Part of what is exported to the ring buffer to the user space reader may be a struct net_device field that is otherwise not available (or at least not yet), hence it's probe-read with mentioned helpers. If you were now to change the SELinux policy for that tc loader application, and therefore replace/swap the progs in the kernel that were loaded with it (given tc's lockdown policy was recorded in their sec blob) with an empty/NULL program, then either you say allow-all or drop-all, but either way, you break the firewalling functionality completely by locking yourself out of the machine or letting everything through. There is no sane way where we could reason about the context/internals of a given program where it would be safe to replace with a simple empty/NULL prog. Best, Daniel