From: "Mickaël Salaün" <mic@digikod.net>
To: enlightened@chromium.org
Cc: linux-security-module@vger.kernel.org, jorgelo@chromium.org,
keescook@chromium.org, groeck@chromium.org, jeffxu@chromium.org,
allenwebb@chromium.org, "Günther Noack" <gnoack3000@gmail.com>,
"Adrian Reber" <areber@redhat.com>,
criu@openvz.org, "Linux API" <linux-api@vger.kernel.org>,
"Jann Horn" <jannh@google.com>,
"Christian Brauner" <brauner@kernel.org>
Subject: Re: [PATCH 0/1] process attribute support for Landlock
Date: Mon, 6 Mar 2023 20:18:00 +0100 [thread overview]
Message-ID: <247f3194-2dd2-1414-0a4d-6e41addf5e64@digikod.net> (raw)
In-Reply-To: <20230302185257.850681-1-enlightened@chromium.org>
Hi Shervin,
Thanks for this initial patch.
On 02/03/2023 19:52, enlightened@chromium.org wrote:
> From: Shervin Oloumi <enlightened@chromium.org>
>
> Hi Mickaël,
>
> I'm looking into adding a simple process attribute getter to Landlock so
> we can determine the sand-boxing state of each process based on
> /proc/[PID]/attr/current. As ChromeOS is expanding Landlock support,
> this would help us paint a clear picture of Landlock coverage in the
> fleet. I prepared a patch as a starting point, and would love to get
> your feedback.
It would help to know exactly what are your needs short term, and long
term. As Günther is wondering, what about nested sandboxing?
I'm thinking about a new /sys/kernel/security/landlock filesystem to be
able to audit Landlock domains (i.e. sandboxes). As for your use case,
it would be useful to be able to tie a process to a Landlock domain
thanks to IDs.
Here are the guiding principles I think would make sense:
1. A sandboxed thread shall not be able to directly know if it is
sandbox nor get any specific information from it's restrictions. The
reason for this principle is to avoid applications to simply jump to
conclusions (and change behavior) if they see that they are sandboxed
with Landlock, instead of trying to access resources and falling back
accordingly. A thread should only be able to inspect its
own/children/nested domains.
2. Access to any Landlock domain information should be checked according
to PTRACE_MODE_READ_FSCREDS, the Landlock domain hierarchy (cf.
ptrace.c:domain_scope_le), and the first principle.
3. Any (domain) ID should be unique to the whole system (or maybe to the
reader's PID namespace, and then in theory relative to the /proc
content) to make it possible to compare Landlock domains (like
/proc/[pid]/ns/* symlinks enable), and avoid trivial races.
4. These IDs should be the same during the whole lifetime of the related
domain.
5. These IDs should not enable to infer information from other Landlock
domains (e.g. how many are in use, current and parent domains), nor the
kernel internals (e.g. addresses).
6. These IDs should not be sequential nor easily guessed to avoid
anti-patterns (cf. file descriptors).
7. These IDs should be CRIU-friendly, to be able to easily restore such
state. This doesn't help the previous principles and I don't know how/if
CRIU supports namespace IDs though.
The /proc/[pid]/ns/* symlinks should be a good inspiration for a
/proc/[pid]/attr/landlock/domain symlink with similar properties. Such
file could then be used to pin or enforce the same Landlock domain on
other threads in the future (out of scope for this patch series). Being
able to open such "domain" file would make it possible to avoid races
while reading the related ID and looking for the related entry in
/sys/kernel/security/landlock/ by holding this file open.
It would be nice if the /proc/[pid]/attr/landlock directory would only
exists if Landlock is enabled.
Similarly, /proc/[pid]/attr/landlock/domain should only exist (or be
viewable) for a thread if [pid] is part of one of its child domain.
For now, I don't see any file in /proc/[pid]/attr/landlock/ other than
"domain" that would make sense, but a dedicated directory is useful anyway.
I though about an entire file hierarchy to reflect a Landlock domain
(e.g., with rule attributes), but that would make the /proc filesystem
dynamically deep, so this should be dedicated to the
/sys/kernel/security/landlock filesystem, but tied with /proc in some
way, in this case with same domain IDs.
>
> One area I am not very sure of is the case where more than one LSM is in
> use. In such cases each LSM could have its own process attribute
> getters and setters. What I learned is that when this is the case, the
> kernel only calls the hook function for the LSM that is loaded first in
> the CONFIG_LSM option. For example if landlock comes first
> (CONFIG_LSM=landlock,...), then the kernel only calls the hook function
> for Landlock, when the userspace interacts with process attribute files.
> This is not a blocker for us, as we only currently care about reading
> the Landlock related attributes, and my understanding is that this is
> working as intended, but wanted to get your input.
Using the /proc/[pid]/attr/landlock/domain path will remove this issue.
>
> Shervin Oloumi (1):
> lsm: adds process attribute getter for Landlock
>
> fs/proc/base.c | 11 +++++++++++
> security/landlock/fs.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+)
>
>
> base-commit: e2ca6ba6ba0152361aa4fcbf6067db71b2c7a770
next parent reply other threads:[~2023-03-06 19:18 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230302185257.850681-1-enlightened@chromium.org>
2023-03-06 19:18 ` Mickaël Salaün [this message]
2023-03-07 14:16 ` [PATCH 0/1] process attribute support for Landlock Mickaël Salaün
2023-03-08 22:25 ` Shervin Oloumi
2023-03-15 9:56 ` Mickaël Salaün
2023-03-16 6:19 ` Günther Noack
2023-03-17 8:38 ` Mickaël Salaün
2023-05-18 20:44 ` Shervin Oloumi
2023-05-24 16:09 ` Mickaël Salaün
2023-05-24 16:21 ` Mickaël Salaün
2023-05-18 20:45 ` [PATCH v2] lsm: adds process attribute getter " Shervin Oloumi
2023-05-18 21:26 ` Casey Schaufler
2023-05-22 19:56 ` Paul Moore
2023-05-23 6:13 ` Jeff Xu
2023-05-23 15:32 ` Casey Schaufler
2023-05-30 18:02 ` Jeff Xu
2023-05-30 19:05 ` Casey Schaufler
2023-05-31 13:01 ` Mickaël Salaün
2023-06-01 20:45 ` Jeff Xu
2023-06-01 21:30 ` Casey Schaufler
2023-05-23 21:12 ` Paul Moore
2023-05-24 15:38 ` Mickaël Salaün
2023-05-24 16:02 ` Mickaël Salaün
2023-05-25 16:28 ` Casey Schaufler
2023-05-30 18:05 ` Jeff Xu
2023-05-30 19:19 ` Casey Schaufler
2023-05-31 13:26 ` Mickaël Salaün
2023-06-01 20:48 ` Jeff Xu
2023-06-01 21:34 ` Casey Schaufler
2023-06-01 22:08 ` Mickaël Salaün
2023-05-24 16:05 ` Mickaël Salaün
2023-05-19 5:22 ` kernel test robot
2023-05-24 16:48 ` Mickaël Salaün
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=247f3194-2dd2-1414-0a4d-6e41addf5e64@digikod.net \
--to=mic@digikod.net \
--cc=allenwebb@chromium.org \
--cc=areber@redhat.com \
--cc=brauner@kernel.org \
--cc=criu@openvz.org \
--cc=enlightened@chromium.org \
--cc=gnoack3000@gmail.com \
--cc=groeck@chromium.org \
--cc=jannh@google.com \
--cc=jeffxu@chromium.org \
--cc=jorgelo@chromium.org \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).