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

[Mickaël Salaün]

On 16/03/2023 07:19, Günther Noack wrote:
> Hi!
> 
> On Wed, Mar 15, 2023 at 10:56:03AM +0100, Mickaël Salaün wrote:
>> 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.
> 
> Fair enough.  I missed the fact that this was about the OS rather than
> the browser.
> 
> Still, out of curiosity: Hypothetically, if you were to expose the
> number of stacked Landlock policies instead of the boolean in that
> place -- would there be any drawbacks to that which I'm overlooking?
> 
> It seems to me, superficially, that the implementation should be
> similarly simple, it would be useful in more cases where Landlock
> users do not have control over the full OS, and I can't currently see
> any cases where having a number instead of a boolean would complicate
> the usage from userspace?  Am I missing something?

I'd like to hear from Shervin, but here is my reasoning.

I'd like to avoid as much as possible the procfs interface (for security 
and usability reasons specific to Landlock), but to only extend it to 
the minimal requirement needed to tie a process to a Landlock domain. 
Exposing any domain information (e.g. nested domain depth) should then 
be managed by a new interface (i.e. /sys/kernel/security/landlock), and 
we should avoid duplicating this information in the procfs interface. 
Making an attr/landlock/domain file gives the information that a 
(nested) domain exists for this PID, which is anyway a required minimal 
interface.


> 
> (But in any case, the boolean is also fine I think.)
> 
> 
>>>> 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.

For a more up-to-date idea, see 
https://lore.kernel.org/all/ee878a04-51f4-a8aa-7d4c-13e519b7409d@digikod.net/
The fdinfo trick would not be required though, I found a better design 
to tie an opened domain to its properties. Anyway, this is future work 
and would be compatible with the /proc/[pid]/attr/landlock/domain file.

> 
> (Small remark:
> 
> Doing anything differently depending on whether and how you are
> landlocked is definitely an antipattern which we should not encourage.
> But I'm not sure whether we can hide the fact very easily.
> 
> It's already possible for a thread to detect whether it is landlocked,
> by using this hack: Create a new thread and then in that thread count
> how many additional sandboxes you can stack on top.
> 
> If you have knowledge about what Landlock configuration you are
> looking for, it will be even easier to detect.
> 
> I hope noone takes the above example as inspiration.)

Indeed, there are multiple ways to detect that a thread is landlocked, 
but we should not make any effort to make it easy to check unless there 
is at least a valid use case. I'd like to only add/show new interfaces 
were/when they are needed, in this case, "a thread should only be able 
to inspect/see its nested domains". For now, the only valid usage I can 
think of to detect sandboxing is for debug and metrics, not for a 
legitimate sandboxed application. Furthermore, what I'd like to have for 
Landlock is the ability to use this "domain" file to get access to 
domain properties (e.g. handled accesses, rules), and giving the sandbox 
configuration to the sandboxed process looks like a bad idea.