Re: [PATCH] security/landlock: use square brackets around "landlock-ruleset"

["Mickaël Salaün" <mic@digikod.net>]

On 12/10/2021 23:09, Paul Moore wrote:
> On Tue, Oct 12, 2021 at 4:38 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>>
>> On Tue, Oct 12, 2021 at 8:12 PM Paul Moore <paul@paul-moore.com> wrote:
>>> On Tue, Oct 12, 2021 at 6:38 AM Christian Brauner
>>> <christian.brauner@ubuntu.com> wrote:
>>>> On Mon, Oct 11, 2021 at 04:38:55PM +0200, Mickaël Salaün wrote:
>>>>> On 11/10/2021 15:37, Christian Brauner wrote:
>>>>>> From: Christian Brauner <christian.brauner@ubuntu.com>
>>>>>>
>>>>>> Make the name of the anon inode fd "[landlock-ruleset]" instead of
>>>>>> "landlock-ruleset". This is minor but most anon inode fds already
>>>>>> carry square brackets around their name:
>>>>>>
>>>>>>     [eventfd]
>>>>>>     [eventpoll]
>>>>>>     [fanotify]
>>>>>>     [fscontext]
>>>>>>     [io_uring]
>>>>>>     [pidfd]
>>>>>>     [signalfd]
>>>>>>     [timerfd]
>>>>>>     [userfaultfd]
>>>>>>
>>>>>> For the sake of consistency lets do the same for the landlock-ruleset anon
>>>>>> inode fd that comes with landlock. We did the same in
>>>>>> 1cdc415f1083 ("uapi, fsopen: use square brackets around "fscontext" [ver #2]")
>>>>>> for the new mount api.
>>>>>
>>>>> Before creating "landlock-ruleset" FD, I looked at other anonymous FD
>>>>> and saw this kind of inconsistency. I don't get why we need to add extra
>>>>> characters to names, those brackets seem useless. If it should be part
>>>>
>>>> Past inconsistency shouldn't justify future inconsistency. If you have a
>>>> strong opinion about this for landlock I'm not going to push for it.
>>>> Exchanging more than 2-3 email about something like this seems too much.
>>>
>>> [NOTE: adding the SELinux list as well as Chris (SELinux refrence
>>> policy maintainer) and Petr (Fedora/RHEL SELinux)]
>>>
>>> Chris and Petr, do either of you currently have any policy that
>>> references the "landlock-ruleset" anonymous inode?  In other words,
>>> would adding the brackets around the name cause you any problems?
>>
>> AFAIU, the anon_inode transitions (the only mechanism where the "file
>> name" would be exposed to the policy) are done only for inodes created
>> by anon_inode_getfd_secure(), which is currently only used by
>> userfaultfd. So you don't even need to ask that question; at this
>> point it should be safe to change any of the names except
>> "[userfaultfd]" as far as SELinux policy is concerned.
> 
> There is also io_uring if you look at selinux/next.
> 
> Regardless, thanks, I didn't check to see if landlock was using the
> new anon inode interface, since both Mickaël and Christian were
> concerned about breaking SELinux I had assumed they were using it :)
> 

Ok, thanks Paul and Ondrej.

Such anonymous inode names seem to be only exposed to proc for now.
Let's change this name then. I think it make sense to backport this
patch down to 5.13 to fix all the inconsistencies.