[CRIU] [PATCH v2] lsm: adds process attribute getter for Landlock

[Casey Schaufler]

On 5/30/2023 11:05 AM, Jeff Xu wrote:
> On Thu, May 25, 2023 at 9:28 AM Casey Schaufler <casey at schaufler-ca.com> wrote:
>> On 5/24/2023 9:02 AM, Mickaël Salaün wrote:
>>> On 24/05/2023 17:38, Mickaël Salaün wrote:
>>>> On 23/05/2023 23:12, Paul Moore wrote:
>>>>> On Tue, May 23, 2023 at 2:13 AM Jeff Xu <jeffxu at chromium.org> wrote:
>>>>>> On Mon, May 22, 2023 at 12:56 PM Paul Moore <paul at paul-moore.com>
>>>>>> wrote:
>>>>>>> On Thu, May 18, 2023 at 5:26 PM Casey Schaufler
>>>>>>> <casey at schaufler-ca.com> wrote:
>>>>>>>> On 5/18/2023 1:45 PM, Shervin Oloumi wrote:
>>>>>>>>> Adds a new getprocattr hook function to the Landlock LSM, which
>>>>>>>>> tracks
>>>>>>>>> the landlocked state of the process. This is invoked when
>>>>>>>>> user-space
>>>>>>>>> reads /proc/[pid]/attr/domain
>>>>>>>> Please don't add a Landlock specific entry directly in the attr/
>>>>>>>> directory. Add it only to attr/landlock.
>>>>>>>>
>>>>>>>> Also be aware that the LSM maintainer (Paul Moore) wants to move
>>>>>>>> away from the /proc/.../attr interfaces in favor of a new system
>>>>>>>> call,
>>>>>>>> which is in review.
>>>>>>> What Casey said above.
>>>>>>>
>>>>>>> There is still some uncertainty around timing, and if we're perfectly
>>>>>>> honest, acceptance of the new syscalls at the Linus level, but yes, I
>>>>>>> would very much like to see the LSM infrastructure move away from
>>>>>>> procfs and towards a syscall API.  Part of the reasoning is that the
>>>>>>> current procfs API is ill-suited to handle the multiple, stacked LSMs
>>>>>>> and the other part being the complexity of procfs in a namespaced
>>>>>>> system.  If the syscall API is ultimately rejected, we will need to
>>>>>>> revisit the idea of a procfs API, but even then I think we'll need to
>>>>>>> make some changes to the current approach.
>>>>>>>
>>>>>>> As I believe we are in the latter stages of review for the syscall
>>>>>>> API, perhaps you could take a look and ensure that the current
>>>>>>> proposed API works for what you are envisioning with Landlock?
>>>> I agree, and since the LSM syscalls are almost ready that should not
>>>> change much the timing. In fact, extending these syscalls might be
>>>> easier than tweaking the current procfs/attr API for Landlock specific
>>>> requirements (e.g. scoped visibility). We should ensure that these
>>>> syscalls would be a good fit to return file descriptors, but in the
>>>> short term we only need to know if a process is landlocked or not, so a
>>>> raw return value (0 or -errno) will be enough.
>>>>
>>>> Mentioning in the LSM syscalls patch series that they may deal with (and
>>>> return) file descriptors could help API reviewers though.
>>> It should be kept in mind that the current LSM syscalls only deal with
>>> the calling task, whereas the goal of this Landlock patch series is to
>>> inspect other tasks. A new LSM syscall would need to be created to
>>> handle pidfd e.g., named lsm_get_proc_attr() or lsm_get_pid_attr().
>> I think it would be lsm_get_pid_attr(). Yes, it's the obvious next step.
>>
>>> I'm not sure if this should be a generic LSM syscall or a Landlock
>>> syscall though. I have plan to handle processes other than the caller
>>> (e.g. to restrict an existing process hierarchy), so thinking about a
>>> Landlock-specific syscall could make sense.
>>>
>>> To summarize, creating a new LSM syscall to deal with pidfd and to get
>>> LSM process "status/attr" looks OK. However, Landlock-specific
>>> syscalls to deal with Landlock specificities (e.g. ruleset or domain
>>> file descriptor) make more sense.
>>>
>>> Having one LSM-generic syscall to get minimal Landlock attributes
>>> (i.e. mainly to know if a process is sandboxed), and another
>>> Landlock-specific syscall to do more things (e.g. get the domain file
>>> descriptor, restrict a task) seems reasonable. The second one would
>>> overlap with the first one though. What do you think?
>> I find it difficult to think of a file descriptor as an attribute of
>> a process. To my (somewhat unorthodox) thinking a file descriptor is
>> a name for an object, not an attribute of the object. You can't access
>> an object by its attributes, but you can by its name. An attribute is
>> a description of the object. I'm perfectly happy with lsm_get_pid_attr()
>> returning an attribute that is a file descriptor if it describes the
>> process in some way, but not as a substitute for opening /proc/42.
>>
>>
> If I understand correctly:
> 1> A new lsm syscall - lsm_get_pid_attr():  Landlock will return the
> process's landlock sandbox status: true/false.

There would have to be a new LSM_ATTR_ENFORCMENT to query.
Each LSM could then report what, if any, value it choose to.
I can't say whether SELinux would take advantage of this.
I don't see that Smack would report this attribute.

>
> Is this a right fit for SELinux to also return the process's enforcing
> mode ? such as enforcing/permissive.
>
> 2> Landlock will have its own specific syscall to deal with Landlock
> specificities (e.g. ruleset or domain file descriptor).

I don't see how a syscall to load arbitrary LSM policy (e.g. landlock ruleset,
Smack rules) would behave, so each LSM is on it's own regarding that. I doubt
that the VFS crowd would be especially keen on an LSM creating file descriptors,
but stranger things have happened.