[PATCH v1] landlock: Explain how to support Landlock

[Mickaël Salaün]

On 13/05/2022 00:03, Paul Moore wrote:
> On Thu, May 12, 2022 at 8:08 AM Mickaël Salaün <mic at digikod.net> wrote:
>>
>> Let's help users by documenting how to enable and check for Landlock in
>> the kernel and the running system.  The userspace-api section may not be
>> the best place for this but it still makes sense to put all the user
>> documentation at the same place.
>>
>> Cc: Paul Moore <paul at paul-moore.com>
>> Signed-off-by: Mickaël Salaün <mic at digikod.net>
>> Link: https://lore.kernel.org/r/20220512120847.124822-1-mic@digikod.net
>> ---
>>   Documentation/userspace-api/landlock.rst | 26 ++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
>> index 7b4fe6218132..e69dbddcc191 100644
>> --- a/Documentation/userspace-api/landlock.rst
>> +++ b/Documentation/userspace-api/landlock.rst
>> @@ -388,6 +388,32 @@ Starting with the Landlock ABI version 2, it is now possible to securely
>>   control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER`
>>   access right.
>>
>> +Landlock support
>> +================
>> +
>> +Kernel configuration
>> +--------------------
> 
> I would suggest changing the section name to "Kernel build
> configuration" to make it clear the instructions below need to be done
> at build time.

That would be better indeed.

[rewinding] …well, this title would go away if I merge this subsection 
with the next one.

> 
>> +Landlock can be supported since Linux 5.13 but it must be configured in the
> 
> I would suggest rephrasing the first part of this sentence slightly:
> 
> "Landlock was first introduced in Linux v5.13 but it must be ..."

OK

> 
>> +kernel with `CONFIG_SECURITY_LANDLOCK=y`, and it should be enabled at boot time
> 
> CONFIG_LSM is a Kconfig build time variable not a runtime command line
> variable, yes?

Yes. This is confusing because it sets the default value of cmdline. 
I'll rephrase this paragraph.

> 
>> +with `CONFIG_LSM=landlock,[...]`.  The `[...]` must be replaced with the list
>> +of LSMs that may be useful for the running system (see the `CONFIG_LSM` help).
>> +
>> +Running system configuration
>> +----------------------------
> 
> This is nit-picky, but I would suggest a section name of "Kernel
> command line configuration".

I was thinking about "Kernel boot configuration" but I'll try to merge 
the two configuration subsections and put the check subsection earlier.

> 
>> +If the running kernel doesn't have `landlock` in `CONFIG_LSM`, then we can
>> +still enable it by adding `lsm=landlock,[...]` to
>> +Documentation/admin-guide/kernel-parameters.rst. The `[...]` must be replaced
>> +with the list of LSMs that may be useful for the running system (see the
>> +`CONFIG_LSM` help).
>> +
>> +Running system check
>> +--------------------
>> +
>> +We can check that Landlock is enabled by looking for "landlock: Up and running"
>> +in kernel logs: `dmesg | grep landlock`
> 
> I would suggest moving the "Running system check" section up closer to
> the top of the documentation, perhaps right before the "Landlock
> rules" section.  My thinking is that it is better to mention this as
> soon as possible in the doc so that users can verify Landlock is
> present and functioning before trying any of the code examples and
> failing.  I think it is okay to leave the "how to enable Landlock"
> sections at the bottom and simply refer to them in the "system check"
> section.
> 

Good idea, thanks!