Message-ID: <97c15e23-8a89-79f2-4413-580153827ade@digikod.net>
Date: Tue, 13 Jun 2023 22:12:08 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>,
 Jeff Xu <jeffxu@...omium.org>, Günther Noack
 <gnoack@...gle.com>
Cc: willemdebruijn.kernel@...il.com, gnoack3000@...il.com,
 linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
 netfilter-devel@...r.kernel.org, yusongping@...wei.com,
 artem.kuzin@...wei.com
Subject: Re: [PATCH v11 12/12] landlock: Document Landlock's network support


On 13/06/2023 12:13, Konstantin Meskhidze (A) wrote:
> 
> 
> 6/7/2023 8:46 AM, Jeff Xu пишет:
>> On Tue, Jun 6, 2023 at 7:09 AM Günther Noack <gnoack@...gle.com> wrote:
>>>
>>> On Tue, May 16, 2023 at 12:13:39AM +0800, Konstantin Meskhidze wrote:
>>>> Describe network access rules for TCP sockets. Add network access
>>>> example in the tutorial. Add kernel configuration support for network.
>>>>
>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>>>> ---
>>>>
>>>> Changes since v10:
>>>> * Fixes documentaion as Mickaёl suggested:
>>>> https://lore.kernel.org/linux-security-module/ec23be77-566e-c8fd-179e-f50e025ac2cf@digikod.net/
>>>>
>>>> Changes since v9:
>>>> * Minor refactoring.
>>>>
>>>> Changes since v8:
>>>> * Minor refactoring.
>>>>
>>>> Changes since v7:
>>>> * Fixes documentaion logic errors and typos as Mickaёl suggested:
>>>> https://lore.kernel.org/netdev/9f354862-2bc3-39ea-92fd-53803d9bbc21@digikod.net/
>>>>
>>>> Changes since v6:
>>>> * Adds network support documentaion.
>>>>
>>>> ---
>>>>   Documentation/userspace-api/landlock.rst | 83 ++++++++++++++++++------
>>>>   1 file changed, 62 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
>>>> index f6a7da21708a..f185dbaa726a 100644
>>>> --- a/Documentation/userspace-api/landlock.rst
>>>> +++ b/Documentation/userspace-api/landlock.rst
>>>> @@ -11,10 +11,10 @@ Landlock: unprivileged access control
>>>>   :Date: October 2022
>>>>
>>>>   The goal of Landlock is to enable to restrict ambient rights (e.g. global
>>>> -filesystem access) for a set of processes.  Because Landlock is a stackable
>>>> -LSM, it makes possible to create safe security sandboxes as new security layers
>>>> -in addition to the existing system-wide access-controls. This kind of sandbox
>>>> -is expected to help mitigate the security impact of bugs or
>>>> +filesystem or network access) for a set of processes.  Because Landlock
>>>> +is a stackable LSM, it makes possible to create safe security sandboxes as new
>>>> +security layers in addition to the existing system-wide access-controls. This
>>>> +kind of sandbox is expected to help mitigate the security impact of bugs or
>>>>   unexpected/malicious behaviors in user space applications.  Landlock empowers
>>>>   any process, including unprivileged ones, to securely restrict themselves.
>>>>
>>>> @@ -28,20 +28,24 @@ appropriately <kernel_support>`.
>>>>   Landlock rules
>>>>   ==============
>>>>
>>>> -A Landlock rule describes an action on an object.  An object is currently a
>>>> -file hierarchy, and the related filesystem actions are defined with `access
>>>> -rights`_.  A set of rules is aggregated in a ruleset, which can then restrict
>>>> -the thread enforcing it, and its future children.
>>>> +A Landlock rule describes an action on a kernel object.  Filesystem
>>>> +objects can be defined with a file hierarchy.  Since the fourth ABI
>>>> +version, TCP ports enable to identify inbound or outbound connections.
>>>> +Actions on these kernel objects are defined according to `access
>>>> +rights`_.  A set of rules is aggregated in a ruleset, which
>>>> +can then restrict the thread enforcing it, and its future children.
>>>
>>> I feel that this paragraph is a bit long-winded to read when the
>>> additional networking aspect is added on top as well.  Maybe it would
>>> be clearer if we spelled it out in a more structured way, splitting up
>>> the filesystem/networking aspects?
>>>
>>> Suggestion:
>>>
>>>    A Landlock rule describes an action on an object which the process
>>>    intends to perform.  A set of rules is aggregated in a ruleset,
>>>    which can then restrict the thread enforcing it, and its future
>>>    children.
>>>
>>>    The two existing types of rules are:
>>>
>>>    Filesystem rules
>>>        For these rules, the object is a file hierarchy,
>>>        and the related filesystem actions are defined with
>>>        `filesystem access rights`.
>>>
>>>    Network rules (since ABI v4)
>>>        For these rules, the object is currently a TCP port,
>> Remote port or local port ?
>>
>     Both ports - remote or local.

Hmm, at first I didn't think it was worth talking about remote or local, 
but I now think it could be less confusing to specify a bit:
"For these rules, the object is the socket identified with a TCP (bind 
or connect) port according to the related `network access rights`."

A port is not a kernel object per see, so I tried to tweak a bit the 
sentence. I'm not sure such detail (object vs. data) would not confuse 
users. Any thought?


>>
>>>        and the related actions are defined with `network access rights`.
>>>
>>> Please note that the landlock(7) man page is in large parts using the
>>> same phrasing as the kernel documentation.  It might be a good idea to
>>> keep them in sync and structured similarly.  (On that mailing list,
>>> the reviews are a bit more focused on good writing style.)
>>>
>>> The same reasoning applies to the example below as well.  Explaining
>>> multiple aspects of a thing in a single example can muddy the message,
>>> let's try to avoid that.  But I can also see that if we had two
>>> separate examples, a large part of the example would be duplicated.

[...]

Powered by blists - more mailing lists