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@xxxxxxxxxxxx> wrote: >>>> On Mon, May 22, 2023 at 12:56 PM Paul Moore <paul@xxxxxxxxxxxxxx> >>>> wrote: >>>>> On Thu, May 18, 2023 at 5:26 PM Casey Schaufler >>>>> <casey@xxxxxxxxxxxxxxxx> 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.
