Message ID | 20230921061641.273654-1-mic@digikod.net |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp5023875vqi; Thu, 21 Sep 2023 10:46:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH3AZWWTrlzMZqQtSFPVISBWABn5YA2lktUo97+wzCSyrpVxDSIC9Zb1eZcLlPnZryrVG3p X-Received: by 2002:a17:902:a5c1:b0:1bc:10cf:50d8 with SMTP id t1-20020a170902a5c100b001bc10cf50d8mr6333146plq.23.1695318376379; Thu, 21 Sep 2023 10:46:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695318376; cv=none; d=google.com; s=arc-20160816; b=iFk07zhDbjtIFCGq+cwPPCNY7xrsi41WUvnPrO7Nlp6DAX5iYXPRDItKNgDPAS+IMQ 87ktnWCDn5Eawx6SNfsgfVx8rcZGOgvhsXLMMH8vUIohUGPjRT/ht+UXtPovooCIyBbK +3ikxJmwoQCAHxnDYF73Q+6+zfUmozgYGyctqIB/Wyf6j1ESb7Qc00McDE1eJO1iVrq8 OiWdie4X4ja72s+rwkSHa2Zads1v/beDdb8EdIPDX91Pcq6Pu6dMtK4f6B3bxFjnmz1a mvkA5UxO9W87XdLjItBsAxfI7wqfJUlyc1ABq2nvchTUWnbrJqAJHT4zzwpOJHkLDERW +kOA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=zmqVVDZHm+4aeCIQ3ZiQEkiVjwVDw4q7xhvYQbEycxc=; fh=dumNNbCJCjvHrFPBOgtHY87pJHoJgAB5Wq5oWhKArCo=; b=ZHfgG/6JtRLfsbZdYZsXcWA/jrG00cU1yj8wyiO4LpZh42ABCvsVeCXcOuuogXg+Yt fCWbY7zcUPZ6DpgGu2UsyCFYi0Dn4yXTHLBdONowV13LfgO3FtsDzgIS7uzV+FCt9/sC pZLfVvJlkCnNn/xGcFqGouX17jOPQnBcDM9mJiMAT/MfN2C6SvWmyxKL1DpX1QjqwAaM yvifm7bQXrn4POoAuCMswrJmLJWmKUfNnF0fLe9/iSqP0lZWMshMGy6QKpopg5zeen6y hPxU/oghaIPOd6Z/hC2b9v7tGAxDCi6LmlbBk6ESE90Ufa0SfEwtCw11Nng2snAHoE+q jG6w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@digikod.net header.s=20191114 header.b=YchhqMwo; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id p20-20020a170902e35400b001b9e31bda39si1776413plc.118.2023.09.21.10.46.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Sep 2023 10:46:16 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@digikod.net header.s=20191114 header.b=YchhqMwo; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id C4C478142D8A; Thu, 21 Sep 2023 10:32:21 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229914AbjIURcR (ORCPT <rfc822;pwkd43@gmail.com> + 29 others); Thu, 21 Sep 2023 13:32:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55068 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230081AbjIURcD (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 21 Sep 2023 13:32:03 -0400 Received: from smtp-190e.mail.infomaniak.ch (smtp-190e.mail.infomaniak.ch [IPv6:2001:1600:4:17::190e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CFD8F10919; Thu, 21 Sep 2023 10:07:32 -0700 (PDT) Received: from smtp-3-0000.mail.infomaniak.ch (unknown [10.4.36.107]) by smtp-3-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4RrlY55m19zMqhBY; Thu, 21 Sep 2023 06:16:57 +0000 (UTC) Received: from unknown by smtp-3-0000.mail.infomaniak.ch (Postfix) with ESMTPA id 4RrlY35QQvz3f; Thu, 21 Sep 2023 08:16:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=digikod.net; s=20191114; t=1695277017; bh=JPGqIw4NybtPv8UXpkiwD6sCB9HCR+gM6q1qZN9IqXk=; h=From:To:Cc:Subject:Date:From; b=YchhqMwoEcrcTmhUUQ9lpZZ8cs3lJqNM4E1jZL1qt4XqjlAFav6irE0qMM/KUEjor M88gN/aDWfE3sWP2nf+DSZjStURCumnvl9CRZ9cj1Y2e4+jYZTFjqbRgUx3GL5+xMj oNyd2c6uILdSsyYJu/YaEeK46TOeELV0lOaM0bdo= From: =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= <mic@digikod.net> To: Eric Paris <eparis@redhat.com>, James Morris <jmorris@namei.org>, Paul Moore <paul@paul-moore.com>, "Serge E . Hallyn" <serge@hallyn.com> Cc: =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= <mic@digikod.net>, Ben Scarlato <akhna@google.com>, =?utf-8?q?G=C3=BCnther_Noack?= <gnoack@google.com>, Jeff Xu <jeffxu@google.com>, Jorge Lucangeli Obes <jorgelo@google.com>, Konstantin Meskhidze <konstantin.meskhidze@huawei.com>, Shervin Oloumi <enlightened@google.com>, audit@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [RFC PATCH v1 0/7] Landlock audit support Date: Thu, 21 Sep 2023 08:16:34 +0200 Message-ID: <20230921061641.273654-1-mic@digikod.net> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Infomaniak-Routing: alpha X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 21 Sep 2023 10:32:21 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777670161522192997 X-GMAIL-MSGID: 1777670161522192997 |
Series |
Landlock audit support
|
|
Message
Mickaël Salaün
Sept. 21, 2023, 6:16 a.m. UTC
Hi, This patch series adds basic audit support to Landlock for most actions. Logging denied requests is useful for different use cases: * app developers: to ease and speed up sandboxing support * power users: to understand denials * sysadmins: to look for users' issues * tailored distro maintainers: to get usage metrics from their fleet * security experts: to detect attack attempts To make logs useful, they need to contain the most relevant Landlock domain that denied an action, and the reason. This translates to the latest nested domain and the related missing access rights. Two "Landlock permissions" are used to describe mandatory restrictions enforced on all domains: * fs_layout: change the view of filesystem with mount operations. * ptrace: tamper with a process. Here is an example of logs, result of the sandboxer activity: tid=267 comm="sandboxer" op=create-ruleset ruleset=1 handled_access_fs=execute,write_file,read_file,read_dir,remove_dir,remove_file,make_char,make_dir,make_reg,make_sock,make_fifo,make_block,make_sym,refer,truncate tid=267 comm="sandboxer" op=restrict-self domain=2 ruleset=1 parent=0 op=release-ruleset ruleset=1 tid=267 comm="bash" domain=2 op=open errno=13 missing-fs-accesses=write_file,read_file missing-permission= path="/dev/tty" dev="devtmpfs" ino=9 tid=268 comm="ls" domain=2 op=open errno=13 missing-fs-accesses=read_dir missing-permission= path="/" dev="vda2" ino=256 tid=269 comm="touch" domain=2 op=mknod errno=13 missing-fs-accesses=make_reg missing-permission= path="/" dev="vda2" ino=256 tid=270 comm="umount" domain=2 op=umount errno=1 missing-fs-accesses= missing-permission=fs_layout name="/" dev="tmpfs" ino=1 tid=271 comm="strace" domain=2 op=ptrace errno=1 missing-fs-accesses= missing-permission=ptrace opid=1 ocomm="systemd" As highlighted in comments, support for audit is not complete yet with this series: some actions are not logged (e.g. file reparenting), and rule additions are not logged neither. I'm also not sure if we need to have seccomp-like features such as SECCOMP_FILTER_FLAG_LOG, SECCOMP_RET_LOG, and /proc/sys/kernel/seccomp/actions_logged I'd like to get some early feedback on this proposal. This series is based on v6.6-rc2 Regards, Mickaël Salaün (7): lsm: Add audit_log_lsm_data() helper landlock: Factor out check_access_path() landlock: Log ruleset creation and release landlock: Log domain creation and enforcement landlock: Log file-related requests landlock: Log mount-related requests landlock: Log ptrace requests include/linux/lsm_audit.h | 2 + include/uapi/linux/audit.h | 1 + security/landlock/Makefile | 2 + security/landlock/audit.c | 283 +++++++++++++++++++++++++++++++++++ security/landlock/audit.h | 88 +++++++++++ security/landlock/fs.c | 169 ++++++++++++++++----- security/landlock/ptrace.c | 47 +++++- security/landlock/ruleset.c | 6 + security/landlock/ruleset.h | 10 ++ security/landlock/syscalls.c | 12 ++ security/lsm_audit.c | 26 ++-- 11 files changed, 595 insertions(+), 51 deletions(-) create mode 100644 security/landlock/audit.c create mode 100644 security/landlock/audit.h base-commit: ce9ecca0238b140b88f43859b211c9fdfd8e5b70
Comments
Hi Mickaël On Wed, Sep 20, 2023 at 11:16 PM Mickaël Salaün <mic@digikod.net> wrote: > > Hi, > > This patch series adds basic audit support to Landlock for most actions. > Logging denied requests is useful for different use cases: > * app developers: to ease and speed up sandboxing support > * power users: to understand denials > * sysadmins: to look for users' issues > * tailored distro maintainers: to get usage metrics from their fleet > * security experts: to detect attack attempts > This is a highly desired feature, I think this will save dev's time when developing Landlock rule sets. Thanks for adding this patch set! -Jeff > To make logs useful, they need to contain the most relevant Landlock > domain that denied an action, and the reason. This translates to the > latest nested domain and the related missing access rights. > > Two "Landlock permissions" are used to describe mandatory restrictions > enforced on all domains: > * fs_layout: change the view of filesystem with mount operations. > * ptrace: tamper with a process. > > Here is an example of logs, result of the sandboxer activity: > tid=267 comm="sandboxer" op=create-ruleset ruleset=1 handled_access_fs=execute,write_file,read_file,read_dir,remove_dir,remove_file,make_char,make_dir,make_reg,make_sock,make_fifo,make_block,make_sym,refer,truncate > tid=267 comm="sandboxer" op=restrict-self domain=2 ruleset=1 parent=0 > op=release-ruleset ruleset=1 > tid=267 comm="bash" domain=2 op=open errno=13 missing-fs-accesses=write_file,read_file missing-permission= path="/dev/tty" dev="devtmpfs" ino=9 > tid=268 comm="ls" domain=2 op=open errno=13 missing-fs-accesses=read_dir missing-permission= path="/" dev="vda2" ino=256 > tid=269 comm="touch" domain=2 op=mknod errno=13 missing-fs-accesses=make_reg missing-permission= path="/" dev="vda2" ino=256 > tid=270 comm="umount" domain=2 op=umount errno=1 missing-fs-accesses= missing-permission=fs_layout name="/" dev="tmpfs" ino=1 > tid=271 comm="strace" domain=2 op=ptrace errno=1 missing-fs-accesses= missing-permission=ptrace opid=1 ocomm="systemd" > > As highlighted in comments, support for audit is not complete yet with > this series: some actions are not logged (e.g. file reparenting), and > rule additions are not logged neither. > > I'm also not sure if we need to have seccomp-like features such as > SECCOMP_FILTER_FLAG_LOG, SECCOMP_RET_LOG, and > /proc/sys/kernel/seccomp/actions_logged > > I'd like to get some early feedback on this proposal. > > This series is based on v6.6-rc2 > > Regards, > > Mickaël Salaün (7): > lsm: Add audit_log_lsm_data() helper > landlock: Factor out check_access_path() > landlock: Log ruleset creation and release > landlock: Log domain creation and enforcement > landlock: Log file-related requests > landlock: Log mount-related requests > landlock: Log ptrace requests > > include/linux/lsm_audit.h | 2 + > include/uapi/linux/audit.h | 1 + > security/landlock/Makefile | 2 + > security/landlock/audit.c | 283 +++++++++++++++++++++++++++++++++++ > security/landlock/audit.h | 88 +++++++++++ > security/landlock/fs.c | 169 ++++++++++++++++----- > security/landlock/ptrace.c | 47 +++++- > security/landlock/ruleset.c | 6 + > security/landlock/ruleset.h | 10 ++ > security/landlock/syscalls.c | 12 ++ > security/lsm_audit.c | 26 ++-- > 11 files changed, 595 insertions(+), 51 deletions(-) > create mode 100644 security/landlock/audit.c > create mode 100644 security/landlock/audit.h > > > base-commit: ce9ecca0238b140b88f43859b211c9fdfd8e5b70 > -- > 2.42.0 >
Hi Mickaël! On Thu, Sep 21, 2023 at 08:16:34AM +0200, Mickaël Salaün wrote: > This patch series adds basic audit support to Landlock for most actions. > Logging denied requests is useful for different use cases: > * app developers: to ease and speed up sandboxing support > * power users: to understand denials > * sysadmins: to look for users' issues > * tailored distro maintainers: to get usage metrics from their fleet > * security experts: to detect attack attempts > > To make logs useful, they need to contain the most relevant Landlock > domain that denied an action, and the reason. This translates to the > latest nested domain and the related missing access rights. Is "domain" always the latest nested domain, or is that the domain which caused the check to fail because it denied the requested access right? (If it is just the counter of how many domains are stacked, this could maybe also be queried through proc instead?) > Two "Landlock permissions" are used to describe mandatory restrictions > enforced on all domains: > * fs_layout: change the view of filesystem with mount operations. > * ptrace: tamper with a process. I find the term "access" already a bit overloaded, and the term "permission" also already appears in other contexts. Maybe we can avoid the additional terminology by grouping these two together in the log format, and calling them the "cause" or "reason" for the deny decision? In a sense, the access rights and the other permissions can already be told apart by their names, so they might also both appear under the same key without causing additional confusion? > Here is an example of logs, result of the sandboxer activity: > tid=267 comm="sandboxer" op=create-ruleset ruleset=1 handled_access_fs=execute,write_file,read_file,read_dir,remove_dir,remove_file,make_char,make_dir,make_reg,make_sock,make_fifo,make_block,make_sym,refer,truncate > tid=267 comm="sandboxer" op=restrict-self domain=2 ruleset=1 parent=0 > op=release-ruleset ruleset=1 > tid=267 comm="bash" domain=2 op=open errno=13 missing-fs-accesses=write_file,read_file missing-permission= path="/dev/tty" dev="devtmpfs" ino=9 > tid=268 comm="ls" domain=2 op=open errno=13 missing-fs-accesses=read_dir missing-permission= path="/" dev="vda2" ino=256 > tid=269 comm="touch" domain=2 op=mknod errno=13 missing-fs-accesses=make_reg missing-permission= path="/" dev="vda2" ino=256 > tid=270 comm="umount" domain=2 op=umount errno=1 missing-fs-accesses= missing-permission=fs_layout name="/" dev="tmpfs" ino=1 > tid=271 comm="strace" domain=2 op=ptrace errno=1 missing-fs-accesses= missing-permission=ptrace opid=1 ocomm="systemd" In more complicated cases like "refer" and "open", it is possible that more than one access right is missing, and presumably they'll both be listed in missing-fs-accesses=. In this case, it is not clear to me whether the domain= number is referring to the first or the second of these missing rights. (Assuming that the domain= is about the domain which caused the denial.) > As highlighted in comments, support for audit is not complete yet with > this series: some actions are not logged (e.g. file reparenting), and > rule additions are not logged neither. When ftruncate(2) gets denied, it is also not possible to tell which of the nested domains is responsible, without additional changes to what we carry around in the file's security blob. (Right now, we calculate the overall truncation right in advance at open(2) time, and just store that bit with the newly opened file.) > I'm also not sure if we need to have seccomp-like features such as > SECCOMP_FILTER_FLAG_LOG, SECCOMP_RET_LOG, and > /proc/sys/kernel/seccomp/actions_logged > > I'd like to get some early feedback on this proposal. If you want to have the full feature set as proposed above for other operations as well, like file reparenting and truncation, it'll complicate the Landlock logic and increase the amount of data that needs to be kept around just for logging. I'm not convinced that this is worth it. After all, the simpler the Landlock implementation is, the easier it'll be to reason about its logic and its security guarantees. A possible simplification would be to omit the domain number which is responsible for a "deny" decision. I feel that for debugging, knowing the fact that Landlock denied an operation might already be a big step forward, and the exact domain responsible for it might not be that important? —Günther
I talked about this patch series at the Kernel Recipes conference, and you might want to take a look at the future work: https://landlock.io/talks/2023-09-25_landlock-audit-kr.pdf In a nutshell, new syscall flags: * For landlock_create_ruleset() to opt-in for logging ruleset-related and domain-related use * For landlock_add_rule() to opt-in for logging this rule if it granted the requested access * For landlock_restrict_self() to opt-in for: * not log anything * handle a permissive mode to log actions that would have been denied (very useful to build a sandbox) On Thu, Sep 21, 2023 at 08:16:34AM +0200, Mickaël Salaün wrote: > Hi, > > This patch series adds basic audit support to Landlock for most actions. > Logging denied requests is useful for different use cases: > * app developers: to ease and speed up sandboxing support > * power users: to understand denials > * sysadmins: to look for users' issues > * tailored distro maintainers: to get usage metrics from their fleet > * security experts: to detect attack attempts > > To make logs useful, they need to contain the most relevant Landlock > domain that denied an action, and the reason. This translates to the > latest nested domain and the related missing access rights. > > Two "Landlock permissions" are used to describe mandatory restrictions > enforced on all domains: > * fs_layout: change the view of filesystem with mount operations. > * ptrace: tamper with a process. > > Here is an example of logs, result of the sandboxer activity: > tid=267 comm="sandboxer" op=create-ruleset ruleset=1 handled_access_fs=execute,write_file,read_file,read_dir,remove_dir,remove_file,make_char,make_dir,make_reg,make_sock,make_fifo,make_block,make_sym,refer,truncate > tid=267 comm="sandboxer" op=restrict-self domain=2 ruleset=1 parent=0 > op=release-ruleset ruleset=1 > tid=267 comm="bash" domain=2 op=open errno=13 missing-fs-accesses=write_file,read_file missing-permission= path="/dev/tty" dev="devtmpfs" ino=9 > tid=268 comm="ls" domain=2 op=open errno=13 missing-fs-accesses=read_dir missing-permission= path="/" dev="vda2" ino=256 > tid=269 comm="touch" domain=2 op=mknod errno=13 missing-fs-accesses=make_reg missing-permission= path="/" dev="vda2" ino=256 > tid=270 comm="umount" domain=2 op=umount errno=1 missing-fs-accesses= missing-permission=fs_layout name="/" dev="tmpfs" ino=1 > tid=271 comm="strace" domain=2 op=ptrace errno=1 missing-fs-accesses= missing-permission=ptrace opid=1 ocomm="systemd" > > As highlighted in comments, support for audit is not complete yet with > this series: some actions are not logged (e.g. file reparenting), and > rule additions are not logged neither. > > I'm also not sure if we need to have seccomp-like features such as > SECCOMP_FILTER_FLAG_LOG, SECCOMP_RET_LOG, and > /proc/sys/kernel/seccomp/actions_logged > > I'd like to get some early feedback on this proposal. > > This series is based on v6.6-rc2 > > Regards, > > Mickaël Salaün (7): > lsm: Add audit_log_lsm_data() helper > landlock: Factor out check_access_path() > landlock: Log ruleset creation and release > landlock: Log domain creation and enforcement > landlock: Log file-related requests > landlock: Log mount-related requests > landlock: Log ptrace requests > > include/linux/lsm_audit.h | 2 + > include/uapi/linux/audit.h | 1 + > security/landlock/Makefile | 2 + > security/landlock/audit.c | 283 +++++++++++++++++++++++++++++++++++ > security/landlock/audit.h | 88 +++++++++++ > security/landlock/fs.c | 169 ++++++++++++++++----- > security/landlock/ptrace.c | 47 +++++- > security/landlock/ruleset.c | 6 + > security/landlock/ruleset.h | 10 ++ > security/landlock/syscalls.c | 12 ++ > security/lsm_audit.c | 26 ++-- > 11 files changed, 595 insertions(+), 51 deletions(-) > create mode 100644 security/landlock/audit.c > create mode 100644 security/landlock/audit.h > > > base-commit: ce9ecca0238b140b88f43859b211c9fdfd8e5b70 > -- > 2.42.0 >
On Tue, Sep 26, 2023 at 06:24:32PM +0200, Günther Noack wrote: > Hi Mickaël! > > On Thu, Sep 21, 2023 at 08:16:34AM +0200, Mickaël Salaün wrote: > > This patch series adds basic audit support to Landlock for most actions. > > Logging denied requests is useful for different use cases: > > * app developers: to ease and speed up sandboxing support > > * power users: to understand denials > > * sysadmins: to look for users' issues > > * tailored distro maintainers: to get usage metrics from their fleet > > * security experts: to detect attack attempts > > > > To make logs useful, they need to contain the most relevant Landlock > > domain that denied an action, and the reason. This translates to the > > latest nested domain and the related missing access rights. > > Is "domain" always the latest nested domain, or is that the domain which caused > the check to fail because it denied the requested access right? (If it is just > the counter of how many domains are stacked, this could maybe also be queried > through proc instead?) The logged domain is the latest nested domain that denied at least one access request (others might be denied by older domains). What do you mean to query it through proc? > > > > Two "Landlock permissions" are used to describe mandatory restrictions > > enforced on all domains: > > * fs_layout: change the view of filesystem with mount operations. > > * ptrace: tamper with a process. > > I find the term "access" already a bit overloaded, and the term "permission" > also already appears in other contexts. Maybe we can avoid the additional > terminology by grouping these two together in the log format, and calling them > the "cause" or "reason" for the deny decision? In a sense, the access rights > and the other permissions can already be told apart by their names, so they > might also both appear under the same key without causing additional confusion? I choose to have two fields (missing-fs-accesses and missing-permission) because one is specific to the FS access rights and the other is generic. The reason of a deny is specifically the "missing FS accesses or the missing permissions". I though about a generic "missing-accesses" but in this case we'll need to prefix all rights with "fs_" or "generic_", which seems too verbose. I think that tying to the access right types would be less confusing when parsing these logs. I'm not a fan of the "permission" name neither, but I didn't find a better name. This comes from EACCES vs. EPERM. BTW, I should use fs_topology instead of fs_layout. > > > > Here is an example of logs, result of the sandboxer activity: > > tid=267 comm="sandboxer" op=create-ruleset ruleset=1 handled_access_fs=execute,write_file,read_file,read_dir,remove_dir,remove_file,make_char,make_dir,make_reg,make_sock,make_fifo,make_block,make_sym,refer,truncate > > tid=267 comm="sandboxer" op=restrict-self domain=2 ruleset=1 parent=0 > > op=release-ruleset ruleset=1 > > tid=267 comm="bash" domain=2 op=open errno=13 missing-fs-accesses=write_file,read_file missing-permission= path="/dev/tty" dev="devtmpfs" ino=9 > > tid=268 comm="ls" domain=2 op=open errno=13 missing-fs-accesses=read_dir missing-permission= path="/" dev="vda2" ino=256 > > tid=269 comm="touch" domain=2 op=mknod errno=13 missing-fs-accesses=make_reg missing-permission= path="/" dev="vda2" ino=256 > > tid=270 comm="umount" domain=2 op=umount errno=1 missing-fs-accesses= missing-permission=fs_layout name="/" dev="tmpfs" ino=1 > > tid=271 comm="strace" domain=2 op=ptrace errno=1 missing-fs-accesses= missing-permission=ptrace opid=1 ocomm="systemd" > > In more complicated cases like "refer" and "open", it is possible that more than > one access right is missing, and presumably they'll both be listed in > missing-fs-accesses=. In this case, it is not clear to me whether the domain= > number is referring to the first or the second of these missing rights. > (Assuming that the domain= is about the domain which caused the denial.) In the case of "open", only the missing access rigths from the youngest domain (that denied at least one request) are printed. This enables to focus on this one, which should be the most common use case and the more useful when debugging a sandbox. This also means that the logs are not complete, only the more relevant informations are logged. It is more complex in the case of "refer" because two paths/objects are involved. I'm not sure how to log such request yet, but I think an useful log entry should contain both the source and the destination paths, which would be new compared to other LSMs. This would require to extend furthermore audit_log_lsm_data(), probably by adding a prefix to the "path=" string. I'd like to log only one entry per denial, and then only one set of missing rights per domain. This should be OK by extracting the youngest missing access rights from the destination and/or the source (according to the same domain). > > > > As highlighted in comments, support for audit is not complete yet with > > this series: some actions are not logged (e.g. file reparenting), and > > rule additions are not logged neither. > > When ftruncate(2) gets denied, it is also not possible to tell which of the > nested domains is responsible, without additional changes to what we carry > around in the file's security blob. (Right now, we calculate the overall > truncation right in advance at open(2) time, and just store that bit with the > newly opened file.) Right, one solution would be to add a pointer to the domain that set the restrictions, but I'd like to avoid that. Instead, we should be able to identify the struct file at open time (another case for logging a granted access), and then delegate the complexity of domain tracking and file lifetime to the (user space) log parser. > > > > I'm also not sure if we need to have seccomp-like features such as > > SECCOMP_FILTER_FLAG_LOG, SECCOMP_RET_LOG, and > > /proc/sys/kernel/seccomp/actions_logged > > > > I'd like to get some early feedback on this proposal. > > If you want to have the full feature set as proposed above for other operations > as well, like file reparenting and truncation, it'll complicate the Landlock > logic and increase the amount of data that needs to be kept around just for > logging. I'm not convinced that this is worth it. After all, the simpler the > Landlock implementation is, the easier it'll be to reason about its logic and > its security guarantees. I'd also like to keep the *enforcement implementation* as simple as possible, and move most of the logging complexity to the audit.c file and user space parsers. This patch series doesn't add complexity to the enforcement logic, only to the audit logic. The amount of data should only be a 64-bit ID per domain and ruleset, and maybe the same for landlock_file_security (but I guess there are other ways to identify a struct file). Being able to debug Landlock policies is a critical feature for its adoption. > > A possible simplification would be to omit the domain number which is > responsible for a "deny" decision. I feel that for debugging, knowing the fact > that Landlock denied an operation might already be a big step forward, and the > exact domain responsible for it might not be that important? Debugging a set of nested policies would be very challenging without the ability to identify the exact domain causing denials. Storing an ID doesn't look like a significant burden isn't it? > > —Günther > > -- > Sent using Mutt 🐕 Woof Woof