[CRIU] [PATCH 0/1] process attribute support for Landlock

[Mickaël Salaün]

On 08/03/2023 23:25, Shervin Oloumi wrote:
> Thanks all for the feedback. This is in reply to Mickaël, but should
> answer Günther's questions as well.
> 
>> It would help to know exactly what are your needs short term, and long
>> term. As Günther is wondering, what about nested sandboxing?
> 
> Our plan is to use the "landlocked" process attribute defined in the
> patch to determine the sandbox state of the system processes and send
> information to our metrics server regarding Landlock coverage. For
> example, the percentage of processes on the system that are sandboxed
> using Landlock.
> 
> Given that we use Landlock in a very specific and controlled way, we
> are not concerned about the inheritance behavior and nested policies,
> at least for the use case of metrics. When daemons are launched in
> ChromiumOS, they have a pre-defined sandboxing configuration that
> dictates whether Landlock should be applied or not. So this attribute
> would help us verify that the processes running on devices in the wild
> indeed have the general sandboxing state that we expect and the
> reality matches our expectation.
> 
> Long-term, it would be useful to learn more information about domains
> and policies through the process attribute interface, but we do not
> currently have a need for that, apart from maybe doing troubleshooting
> when defining Landlock rules for system daemons.

OK, it makes sense.


> 
>> 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.
> 
> I think this goes beyond the scope for our current needs, but
> certainly a nice feature that we could potentially use in the future.
> So given this, I was wondering what would be the minimum changes we
> can make now (if any) that would serve our purpose AND would be
> compatible with your long-term vision, without getting too deep into
> the implementation of broader concepts. We are flexible on the
> approach for querying the landlocked property (for example whether it
> is based on the presence of a /proc/.../attr/domain or actually
> reading an attribute).

Yes, the approach I suggested, check the /proc/.../attr/landlock/domain 
presence would enable you to check the landlocked state of a process. It 
should not change much from your initial patch. In fact it will be 
quicker to check because there is no need for the open/read/close 
syscalls, but only faccessat2.


>> 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.
> 
> One thing worth noting is that we use a system daemon to read process
> attributes. We have the ptrace_scope set to 1 and the daemon reading
> the attributes does have cap_sys_ptrace, however it is not related to
> the other processes on the system. Do you see this as a problem given
> principle#1?

That should work fine because your deamon is more privileged than the 
checked processes.


>> 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.
> 
> Since these points are regarding the properties of the domain IDs,
> they should not interfere with anything we would implement for
> determining the process sandbox status in any initial patch, but are
> good to know.
> 
>> It would be nice if the /proc/[pid]/attr/landlock directory would only
>> exists if Landlock is enabled.
> 
> This is the current default behavior I believe.
> 
>> 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.
> 
> I am not sure if this is a blocker for our model of a single daemon
> querying the attribute for all processes. Are you suggesting that the
> file would not exist from the view of the other processes if they are
> not the parent process?

Not the parent process, but a parent domain, *or in no domain at all*, 
which is your case.

> 
>> 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.
> 
> Determining the sandbox status of processes based on the existence of
> /proc/[pid]/landlock/domain would serve our simple use case, pending
> the open questions/potential blockers above and a clarification on
> minimum requirements for an initial version.

It should be fine for all these use cases, and only requires a small set 
of changes for now.