[v11,12/12] landlock: Document Landlock's network support

Commit Message

Konstantin Meskhidze (A) May 15, 2023, 4:13 p.m. UTC

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 <[e-mail]>
---

Changes since v10:
* Fixes documentaion as Mickaёl suggested:
https://lore.kernel.org/linux-security-module/[e-mail]/

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/[e-mail]/

Changes since v6:
* Adds network support documentaion.

---
 Documentation/userspace-api/landlock.rst | 83 ++++++++++++++++++------
 1 file changed, 62 insertions(+), 21 deletions(-)

--
2.25.1

Comments

Günther Noack June 6, 2023, 2:08 p.m. UTC | #1

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 <[e-mail]>
> ---
> 
> Changes since v10:
> * Fixes documentaion as Mickaёl suggested:
> https://lore.kernel.org/linux-security-module/[e-mail]/
> 
> 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/[e-mail]/
> 
> 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,
      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.

>  Defining and enforcing a security policy
>  ----------------------------------------
> 
>  We first need to define the ruleset that will contain our rules.  For this
> -example, the ruleset will contain rules that only allow read actions, but write
> -actions will be denied.  The ruleset then needs to handle both of these kind of
> -actions.  This is required for backward and forward compatibility (i.e. the
> -kernel and user space may not know each other's supported restrictions), hence
> -the need to be explicit about the denied-by-default access rights.
> +example, the ruleset will contain rules that only allow filesystem read actions
> +and establish a specific TCP connection, but filesystem write actions
> +and other TCP actions will be denied.  The ruleset then needs to handle both of
> +these kind of actions.  This is required for backward and forward compatibility
> +(i.e. the kernel and user space may not know each other's supported
> +restrictions), hence the need to be explicit about the denied-by-default access
> +rights.

I think it became a bit long - I'd suggest to split it into multiple
paragraphs, one after "our rules." (in line with landlock(7)), and one
after "will be denied."

Maybe the long sentence "For this example, ..." in the middle
paragraph could also be split up in two, to make it more readable?  I
think the point of that sentence is really just to give a brief
overview over what ruleset we are setting out to write.

> 
>  .. code-block:: c
> 
> @@ -62,6 +66,9 @@ the need to be explicit about the denied-by-default access rights.
>              LANDLOCK_ACCESS_FS_MAKE_SYM |
>              LANDLOCK_ACCESS_FS_REFER |
>              LANDLOCK_ACCESS_FS_TRUNCATE,
> +        .handled_access_net =
> +            LANDLOCK_ACCESS_NET_BIND_TCP |
> +            LANDLOCK_ACCESS_NET_CONNECT_TCP,
>      };
> 
>  Because we may not know on which kernel version an application will be
> @@ -70,14 +77,18 @@ should try to protect users as much as possible whatever the kernel they are
>  using.  To avoid binary enforcement (i.e. either all security features or
>  none), we can leverage a dedicated Landlock command to get the current version
>  of the Landlock ABI and adapt the handled accesses.  Let's check if we should
> -remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
> -access rights, which are only supported starting with the second and third
> -version of the ABI.
> +remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE`` or
> +network access rights, which are only supported starting with the second,
> +third and fourth version of the ABI.

At some point it becomes too much to spell it out in one sentence; I'd recommend

  Let's check if we should remove access rights which are only supported
  in higher versions of the ABI.

> 
>  .. code-block:: c
> 
>      int abi;
> 
> +    #define ACCESS_NET_BIND_CONNECT ( \
> +        LANDLOCK_ACCESS_NET_BIND_TCP | \
> +        LANDLOCK_ACCESS_NET_CONNECT_TCP)
> +

This #define does not seem to be used? -- Drop it?


>      abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
>      if (abi < 0) {
>          /* Degrades gracefully if Landlock is not handled. */
> @@ -92,6 +103,11 @@ version of the ABI.
>      case 2:
>          /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
>          ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
> +    case 3:
> +        /* Removes network support for ABI < 4 */
> +        ruleset_attr.handled_access_net &=
> +            ~(LANDLOCK_ACCESS_NET_BIND_TCP |
> +              LANDLOCK_ACCESS_NET_CONNECT_TCP);
>      }
> 
>  This enables to create an inclusive ruleset that will contain our rules.
> @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
>  ABI version.  In this example, this is not required because all of the requested
>  ``allowed_access`` rights are already available in ABI 1.
> 
> -We now have a ruleset with one rule allowing read access to ``/usr`` while
> -denying all other handled accesses for the filesystem.  The next step is to
> -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
> -binary).
> +For network access-control, we can add a set of rules that allow to use a port
> +number for a specific action: HTTPS connections.
> +
> +.. code-block:: c
> +
> +    struct landlock_net_service_attr net_service = {
> +        .allowed_access = NET_CONNECT_TCP,
> +        .port = 443,
> +    };
> +
> +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +                            &net_service, 0);
> +
> +The next step is to restrict the current thread from gaining more privileges
> +(e.g. through a SUID binary). We now have a ruleset with the first rule allowing
> +read access to ``/usr`` while denying all other handled accesses for the filesystem,
> +and a second rule allowing HTTPS connections.
> 
>  .. code-block:: c
> 
> @@ -355,7 +384,7 @@ Access rights
>  -------------
> 
>  .. kernel-doc:: include/uapi/linux/landlock.h
> -    :identifiers: fs_access
> +    :identifiers: fs_access net_access
> 
>  Creating a new ruleset
>  ----------------------
> @@ -374,6 +403,7 @@ Extending a ruleset
> 
>  .. kernel-doc:: include/uapi/linux/landlock.h
>      :identifiers: landlock_rule_type landlock_path_beneath_attr
> +                  landlock_net_service_attr
> 
>  Enforcing a ruleset
>  -------------------
> @@ -451,6 +481,12 @@ always allowed when using a kernel that only supports the first or second ABI.
>  Starting with the Landlock ABI version 3, it is now possible to securely control
>  truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.
> 
> +Network support (ABI < 4)
> +-------------------------
> +
> +Starting with the Landlock ABI version 4, it is now possible to restrict TCP
> +bind and connect actions to only a set of allowed ports.
> +
>  .. _kernel_support:
> 
>  Kernel support
> @@ -469,6 +505,11 @@ still enable it by adding ``lsm=landlock,[...]`` to
>  Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>  configuration.
> 
> +To be able to explicitly allow TCP operations (e.g., adding a network rule with
> +``LANDLOCK_ACCESS_NET_TCP_BIND``), the kernel must support TCP (``CONFIG_INET=y``).
> +Otherwise, sys_landlock_add_rule() returns an ``EAFNOSUPPORT`` error, which can
> +safely be ignored because this kind of TCP operation is already not possible.
> +
>  Questions and answers
>  =====================
> 
> --
> 2.25.1
> 

—Günther

Jeff Xu June 7, 2023, 5:46 a.m. UTC | #2

On Tue, Jun 6, 2023 at 7:09 AM Günther Noack <[e-mail]> 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 <[e-mail]>
> > ---
> >
> > Changes since v10:
> > * Fixes documentaion as Mickaёl suggested:
> > https://lore.kernel.org/linux-security-module/[e-mail]/
> >
> > 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/[e-mail]/
> >
> > 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 ?


>       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.
>
> >  Defining and enforcing a security policy
> >  ----------------------------------------
> >
> >  We first need to define the ruleset that will contain our rules.  For this
> > -example, the ruleset will contain rules that only allow read actions, but write
> > -actions will be denied.  The ruleset then needs to handle both of these kind of
> > -actions.  This is required for backward and forward compatibility (i.e. the
> > -kernel and user space may not know each other's supported restrictions), hence
> > -the need to be explicit about the denied-by-default access rights.
> > +example, the ruleset will contain rules that only allow filesystem read actions
> > +and establish a specific TCP connection, but filesystem write actions
> > +and other TCP actions will be denied.  The ruleset then needs to handle both of
> > +these kind of actions.  This is required for backward and forward compatibility
> > +(i.e. the kernel and user space may not know each other's supported
> > +restrictions), hence the need to be explicit about the denied-by-default access
> > +rights.
>
> I think it became a bit long - I'd suggest to split it into multiple
> paragraphs, one after "our rules." (in line with landlock(7)), and one
> after "will be denied."
>
> Maybe the long sentence "For this example, ..." in the middle
> paragraph could also be split up in two, to make it more readable?  I
> think the point of that sentence is really just to give a brief
> overview over what ruleset we are setting out to write.
>
> >
> >  .. code-block:: c
> >
> > @@ -62,6 +66,9 @@ the need to be explicit about the denied-by-default access rights.
> >              LANDLOCK_ACCESS_FS_MAKE_SYM |
> >              LANDLOCK_ACCESS_FS_REFER |
> >              LANDLOCK_ACCESS_FS_TRUNCATE,
> > +        .handled_access_net =
> > +            LANDLOCK_ACCESS_NET_BIND_TCP |
> > +            LANDLOCK_ACCESS_NET_CONNECT_TCP,
> >      };
> >
> >  Because we may not know on which kernel version an application will be
> > @@ -70,14 +77,18 @@ should try to protect users as much as possible whatever the kernel they are
> >  using.  To avoid binary enforcement (i.e. either all security features or
> >  none), we can leverage a dedicated Landlock command to get the current version
> >  of the Landlock ABI and adapt the handled accesses.  Let's check if we should
> > -remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
> > -access rights, which are only supported starting with the second and third
> > -version of the ABI.
> > +remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE`` or
> > +network access rights, which are only supported starting with the second,
> > +third and fourth version of the ABI.
>
> At some point it becomes too much to spell it out in one sentence; I'd recommend
>
>   Let's check if we should remove access rights which are only supported
>   in higher versions of the ABI.
>
> >
> >  .. code-block:: c
> >
> >      int abi;
> >
> > +    #define ACCESS_NET_BIND_CONNECT ( \
> > +        LANDLOCK_ACCESS_NET_BIND_TCP | \
> > +        LANDLOCK_ACCESS_NET_CONNECT_TCP)
> > +
>
> This #define does not seem to be used? -- Drop it?
>
>
> >      abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
> >      if (abi < 0) {
> >          /* Degrades gracefully if Landlock is not handled. */
> > @@ -92,6 +103,11 @@ version of the ABI.
> >      case 2:
> >          /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
> >          ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
> > +    case 3:
> > +        /* Removes network support for ABI < 4 */
> > +        ruleset_attr.handled_access_net &=
> > +            ~(LANDLOCK_ACCESS_NET_BIND_TCP |
> > +              LANDLOCK_ACCESS_NET_CONNECT_TCP);
> >      }
> >
> >  This enables to create an inclusive ruleset that will contain our rules.
> > @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
> >  ABI version.  In this example, this is not required because all of the requested
> >  ``allowed_access`` rights are already available in ABI 1.
> >
> > -We now have a ruleset with one rule allowing read access to ``/usr`` while
> > -denying all other handled accesses for the filesystem.  The next step is to
> > -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
> > -binary).
> > +For network access-control, we can add a set of rules that allow to use a port
> > +number for a specific action: HTTPS connections.
> > +
> > +.. code-block:: c
> > +
> > +    struct landlock_net_service_attr net_service = {
> > +        .allowed_access = NET_CONNECT_TCP,
> > +        .port = 443,
> > +    };
> > +
> > +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> > +                            &net_service, 0);
> > +
> > +The next step is to restrict the current thread from gaining more privileges
> > +(e.g. through a SUID binary). We now have a ruleset with the first rule allowing
> > +read access to ``/usr`` while denying all other handled accesses for the filesystem,
> > +and a second rule allowing HTTPS connections.
> >
> >  .. code-block:: c
> >
> > @@ -355,7 +384,7 @@ Access rights
> >  -------------
> >
> >  .. kernel-doc:: include/uapi/linux/landlock.h
> > -    :identifiers: fs_access
> > +    :identifiers: fs_access net_access
> >
> >  Creating a new ruleset
> >  ----------------------
> > @@ -374,6 +403,7 @@ Extending a ruleset
> >
> >  .. kernel-doc:: include/uapi/linux/landlock.h
> >      :identifiers: landlock_rule_type landlock_path_beneath_attr
> > +                  landlock_net_service_attr
> >
> >  Enforcing a ruleset
> >  -------------------
> > @@ -451,6 +481,12 @@ always allowed when using a kernel that only supports the first or second ABI.
> >  Starting with the Landlock ABI version 3, it is now possible to securely control
> >  truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.
> >
> > +Network support (ABI < 4)
> > +-------------------------
> > +
> > +Starting with the Landlock ABI version 4, it is now possible to restrict TCP
> > +bind and connect actions to only a set of allowed ports.
> > +
> >  .. _kernel_support:
> >
> >  Kernel support
> > @@ -469,6 +505,11 @@ still enable it by adding ``lsm=landlock,[...]`` to
> >  Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
> >  configuration.
> >
> > +To be able to explicitly allow TCP operations (e.g., adding a network rule with
> > +``LANDLOCK_ACCESS_NET_TCP_BIND``), the kernel must support TCP (``CONFIG_INET=y``).
> > +Otherwise, sys_landlock_add_rule() returns an ``EAFNOSUPPORT`` error, which can
> > +safely be ignored because this kind of TCP operation is already not possible.
> > +
> >  Questions and answers
> >  =====================
> >
> > --
> > 2.25.1
> >
>
> —Günther
>
> --
> Sent using Mutt 

Konstantin Meskhidze (A) June 13, 2023, 10:13 a.m. UTC | #3

6/7/2023 8:46 AM, Jeff Xu пишет:
> On Tue, Jun 6, 2023 at 7:09 AM Günther Noack <[e-mail]> 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 <[e-mail]>
>> > ---
>> >
>> > Changes since v10:
>> > * Fixes documentaion as Mickaёl suggested:
>> > https://lore.kernel.org/linux-security-module/[e-mail]/
>> >
>> > 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/[e-mail]/
>> >
>> > 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.
> 
>>       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.
>>
>> >  Defining and enforcing a security policy
>> >  ----------------------------------------
>> >
>> >  We first need to define the ruleset that will contain our rules.  For this
>> > -example, the ruleset will contain rules that only allow read actions, but write
>> > -actions will be denied.  The ruleset then needs to handle both of these kind of
>> > -actions.  This is required for backward and forward compatibility (i.e. the
>> > -kernel and user space may not know each other's supported restrictions), hence
>> > -the need to be explicit about the denied-by-default access rights.
>> > +example, the ruleset will contain rules that only allow filesystem read actions
>> > +and establish a specific TCP connection, but filesystem write actions
>> > +and other TCP actions will be denied.  The ruleset then needs to handle both of
>> > +these kind of actions.  This is required for backward and forward compatibility
>> > +(i.e. the kernel and user space may not know each other's supported
>> > +restrictions), hence the need to be explicit about the denied-by-default access
>> > +rights.
>>
>> I think it became a bit long - I'd suggest to split it into multiple
>> paragraphs, one after "our rules." (in line with landlock(7)), and one
>> after "will be denied."
>>
>> Maybe the long sentence "For this example, ..." in the middle
>> paragraph could also be split up in two, to make it more readable?  I
>> think the point of that sentence is really just to give a brief
>> overview over what ruleset we are setting out to write.
>>
>> >
>> >  .. code-block:: c
>> >
>> > @@ -62,6 +66,9 @@ the need to be explicit about the denied-by-default access rights.
>> >              LANDLOCK_ACCESS_FS_MAKE_SYM |
>> >              LANDLOCK_ACCESS_FS_REFER |
>> >              LANDLOCK_ACCESS_FS_TRUNCATE,
>> > +        .handled_access_net =
>> > +            LANDLOCK_ACCESS_NET_BIND_TCP |
>> > +            LANDLOCK_ACCESS_NET_CONNECT_TCP,
>> >      };
>> >
>> >  Because we may not know on which kernel version an application will be
>> > @@ -70,14 +77,18 @@ should try to protect users as much as possible whatever the kernel they are
>> >  using.  To avoid binary enforcement (i.e. either all security features or
>> >  none), we can leverage a dedicated Landlock command to get the current version
>> >  of the Landlock ABI and adapt the handled accesses.  Let's check if we should
>> > -remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
>> > -access rights, which are only supported starting with the second and third
>> > -version of the ABI.
>> > +remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE`` or
>> > +network access rights, which are only supported starting with the second,
>> > +third and fourth version of the ABI.
>>
>> At some point it becomes too much to spell it out in one sentence; I'd recommend
>>
>>   Let's check if we should remove access rights which are only supported
>>   in higher versions of the ABI.
>>
>> >
>> >  .. code-block:: c
>> >
>> >      int abi;
>> >
>> > +    #define ACCESS_NET_BIND_CONNECT ( \
>> > +        LANDLOCK_ACCESS_NET_BIND_TCP | \
>> > +        LANDLOCK_ACCESS_NET_CONNECT_TCP)
>> > +
>>
>> This #define does not seem to be used? -- Drop it?
>>
>>
>> >      abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
>> >      if (abi < 0) {
>> >          /* Degrades gracefully if Landlock is not handled. */
>> > @@ -92,6 +103,11 @@ version of the ABI.
>> >      case 2:
>> >          /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
>> >          ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
>> > +    case 3:
>> > +        /* Removes network support for ABI < 4 */
>> > +        ruleset_attr.handled_access_net &=
>> > +            ~(LANDLOCK_ACCESS_NET_BIND_TCP |
>> > +              LANDLOCK_ACCESS_NET_CONNECT_TCP);
>> >      }
>> >
>> >  This enables to create an inclusive ruleset that will contain our rules.
>> > @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
>> >  ABI version.  In this example, this is not required because all of the requested
>> >  ``allowed_access`` rights are already available in ABI 1.
>> >
>> > -We now have a ruleset with one rule allowing read access to ``/usr`` while
>> > -denying all other handled accesses for the filesystem.  The next step is to
>> > -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
>> > -binary).
>> > +For network access-control, we can add a set of rules that allow to use a port
>> > +number for a specific action: HTTPS connections.
>> > +
>> > +.. code-block:: c
>> > +
>> > +    struct landlock_net_service_attr net_service = {
>> > +        .allowed_access = NET_CONNECT_TCP,
>> > +        .port = 443,
>> > +    };
>> > +
>> > +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> > +                            &net_service, 0);
>> > +
>> > +The next step is to restrict the current thread from gaining more privileges
>> > +(e.g. through a SUID binary). We now have a ruleset with the first rule allowing
>> > +read access to ``/usr`` while denying all other handled accesses for the filesystem,
>> > +and a second rule allowing HTTPS connections.
>> >
>> >  .. code-block:: c
>> >
>> > @@ -355,7 +384,7 @@ Access rights
>> >  -------------
>> >
>> >  .. kernel-doc:: include/uapi/linux/landlock.h
>> > -    :identifiers: fs_access
>> > +    :identifiers: fs_access net_access
>> >
>> >  Creating a new ruleset
>> >  ----------------------
>> > @@ -374,6 +403,7 @@ Extending a ruleset
>> >
>> >  .. kernel-doc:: include/uapi/linux/landlock.h
>> >      :identifiers: landlock_rule_type landlock_path_beneath_attr
>> > +                  landlock_net_service_attr
>> >
>> >  Enforcing a ruleset
>> >  -------------------
>> > @@ -451,6 +481,12 @@ always allowed when using a kernel that only supports the first or second ABI.
>> >  Starting with the Landlock ABI version 3, it is now possible to securely control
>> >  truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.
>> >
>> > +Network support (ABI < 4)
>> > +-------------------------
>> > +
>> > +Starting with the Landlock ABI version 4, it is now possible to restrict TCP
>> > +bind and connect actions to only a set of allowed ports.
>> > +
>> >  .. _kernel_support:
>> >
>> >  Kernel support
>> > @@ -469,6 +505,11 @@ still enable it by adding ``lsm=landlock,[...]`` to
>> >  Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>> >  configuration.
>> >
>> > +To be able to explicitly allow TCP operations (e.g., adding a network rule with
>> > +``LANDLOCK_ACCESS_NET_TCP_BIND``), the kernel must support TCP (``CONFIG_INET=y``).
>> > +Otherwise, sys_landlock_add_rule() returns an ``EAFNOSUPPORT`` error, which can
>> > +safely be ignored because this kind of TCP operation is already not possible.
>> > +
>> >  Questions and answers
>> >  =====================
>> >
>> > --
>> > 2.25.1
>> >
>>
>> —Günther
>>
>> --
>> Sent using Mutt 

Mickaël Salaün June 13, 2023, 7:56 p.m. UTC | #4

Thanks Günther, I agree with your review.

On 06/06/2023 16:08, Günther Noack 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 <[e-mail]>
>> ---
>>
>> Changes since v10:
>> * Fixes documentaion as Mickaёl suggested:
>> https://lore.kernel.org/linux-security-module/[e-mail]/
>>
>> 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/[e-mail]/
>>
>> 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,
>        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.
> 
>>   Defining and enforcing a security policy
>>   ----------------------------------------
>>
>>   We first need to define the ruleset that will contain our rules.  For this
>> -example, the ruleset will contain rules that only allow read actions, but write
>> -actions will be denied.  The ruleset then needs to handle both of these kind of
>> -actions.  This is required for backward and forward compatibility (i.e. the
>> -kernel and user space may not know each other's supported restrictions), hence
>> -the need to be explicit about the denied-by-default access rights.
>> +example, the ruleset will contain rules that only allow filesystem read actions
>> +and establish a specific TCP connection, but filesystem write actions
>> +and other TCP actions will be denied.  The ruleset then needs to handle both of
>> +these kind of actions.  This is required for backward and forward compatibility
>> +(i.e. the kernel and user space may not know each other's supported
>> +restrictions), hence the need to be explicit about the denied-by-default access
>> +rights.
> 
> I think it became a bit long - I'd suggest to split it into multiple
> paragraphs, one after "our rules." (in line with landlock(7)), and one
> after "will be denied."
> 
> Maybe the long sentence "For this example, ..." in the middle
> paragraph could also be split up in two, to make it more readable?  I
> think the point of that sentence is really just to give a brief
> overview over what ruleset we are setting out to write.
> 
>>
>>   .. code-block:: c
>>
>> @@ -62,6 +66,9 @@ the need to be explicit about the denied-by-default access rights.
>>               LANDLOCK_ACCESS_FS_MAKE_SYM |
>>               LANDLOCK_ACCESS_FS_REFER |
>>               LANDLOCK_ACCESS_FS_TRUNCATE,
>> +        .handled_access_net =
>> +            LANDLOCK_ACCESS_NET_BIND_TCP |
>> +            LANDLOCK_ACCESS_NET_CONNECT_TCP,
>>       };
>>
>>   Because we may not know on which kernel version an application will be
>> @@ -70,14 +77,18 @@ should try to protect users as much as possible whatever the kernel they are
>>   using.  To avoid binary enforcement (i.e. either all security features or
>>   none), we can leverage a dedicated Landlock command to get the current version
>>   of the Landlock ABI and adapt the handled accesses.  Let's check if we should
>> -remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
>> -access rights, which are only supported starting with the second and third
>> -version of the ABI.
>> +remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE`` or
>> +network access rights, which are only supported starting with the second,
>> +third and fourth version of the ABI.
> 
> At some point it becomes too much to spell it out in one sentence; I'd recommend
> 
>    Let's check if we should remove access rights which are only supported
>    in higher versions of the ABI.
> 
>>
>>   .. code-block:: c
>>
>>       int abi;
>>
>> +    #define ACCESS_NET_BIND_CONNECT ( \
>> +        LANDLOCK_ACCESS_NET_BIND_TCP | \
>> +        LANDLOCK_ACCESS_NET_CONNECT_TCP)
>> +
> 
> This #define does not seem to be used? -- Drop it?
> 
> 
>>       abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
>>       if (abi < 0) {
>>           /* Degrades gracefully if Landlock is not handled. */
>> @@ -92,6 +103,11 @@ version of the ABI.
>>       case 2:
>>           /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
>>           ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
>> +    case 3:
>> +        /* Removes network support for ABI < 4 */
>> +        ruleset_attr.handled_access_net &=
>> +            ~(LANDLOCK_ACCESS_NET_BIND_TCP |
>> +              LANDLOCK_ACCESS_NET_CONNECT_TCP);
>>       }
>>
>>   This enables to create an inclusive ruleset that will contain our rules.
>> @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
>>   ABI version.  In this example, this is not required because all of the requested
>>   ``allowed_access`` rights are already available in ABI 1.
>>
>> -We now have a ruleset with one rule allowing read access to ``/usr`` while
>> -denying all other handled accesses for the filesystem.  The next step is to
>> -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
>> -binary).
>> +For network access-control, we can add a set of rules that allow to use a port
>> +number for a specific action: HTTPS connections.
>> +
>> +.. code-block:: c
>> +
>> +    struct landlock_net_service_attr net_service = {
>> +        .allowed_access = NET_CONNECT_TCP,
>> +        .port = 443,
>> +    };
>> +
>> +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> +                            &net_service, 0);
>> +
>> +The next step is to restrict the current thread from gaining more privileges
>> +(e.g. through a SUID binary). We now have a ruleset with the first rule allowing
>> +read access to ``/usr`` while denying all other handled accesses for the filesystem,
>> +and a second rule allowing HTTPS connections.
>>
>>   .. code-block:: c
>>
>> @@ -355,7 +384,7 @@ Access rights
>>   -------------
>>
>>   .. kernel-doc:: include/uapi/linux/landlock.h
>> -    :identifiers: fs_access
>> +    :identifiers: fs_access net_access
>>
>>   Creating a new ruleset
>>   ----------------------
>> @@ -374,6 +403,7 @@ Extending a ruleset
>>
>>   .. kernel-doc:: include/uapi/linux/landlock.h
>>       :identifiers: landlock_rule_type landlock_path_beneath_attr
>> +                  landlock_net_service_attr
>>
>>   Enforcing a ruleset
>>   -------------------
>> @@ -451,6 +481,12 @@ always allowed when using a kernel that only supports the first or second ABI.
>>   Starting with the Landlock ABI version 3, it is now possible to securely control
>>   truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.
>>
>> +Network support (ABI < 4)
>> +-------------------------
>> +
>> +Starting with the Landlock ABI version 4, it is now possible to restrict TCP
>> +bind and connect actions to only a set of allowed ports.
>> +
>>   .. _kernel_support:
>>
>>   Kernel support
>> @@ -469,6 +505,11 @@ still enable it by adding ``lsm=landlock,[...]`` to
>>   Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>>   configuration.
>>
>> +To be able to explicitly allow TCP operations (e.g., adding a network rule with
>> +``LANDLOCK_ACCESS_NET_TCP_BIND``), the kernel must support TCP (``CONFIG_INET=y``).
>> +Otherwise, sys_landlock_add_rule() returns an ``EAFNOSUPPORT`` error, which can
>> +safely be ignored because this kind of TCP operation is already not possible.
>> +
>>   Questions and answers
>>   =====================
>>
>> --
>> 2.25.1
>>
> 
> —Günther
>

Mickaël Salaün June 13, 2023, 8:12 p.m. UTC | #5

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 <[e-mail]> 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 <[e-mail]>
>>>> ---
>>>>
>>>> Changes since v10:
>>>> * Fixes documentaion as Mickaёl suggested:
>>>> https://lore.kernel.org/linux-security-module/[e-mail]/
>>>>
>>>> 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/[e-mail]/
>>>>
>>>> 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.

[...]

Konstantin Meskhidze (A) June 19, 2023, 2:25 p.m. UTC | #6

6/13/2023 10:56 PM, Mickaël Salaün пишет:
> Thanks Günther, I agree with your review.

   Thanks Günther, I will fix documentation.
> 
> On 06/06/2023 16:08, Günther Noack 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 <[e-mail]>
>>> ---
>>>
>>> Changes since v10:
>>> * Fixes documentaion as Mickaёl suggested:
>>> https://lore.kernel.org/linux-security-module/[e-mail]/
>>>
>>> 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/[e-mail]/
>>>
>>> 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,
>>        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.
>> 
>>>   Defining and enforcing a security policy
>>>   ----------------------------------------
>>>
>>>   We first need to define the ruleset that will contain our rules.  For this
>>> -example, the ruleset will contain rules that only allow read actions, but write
>>> -actions will be denied.  The ruleset then needs to handle both of these kind of
>>> -actions.  This is required for backward and forward compatibility (i.e. the
>>> -kernel and user space may not know each other's supported restrictions), hence
>>> -the need to be explicit about the denied-by-default access rights.
>>> +example, the ruleset will contain rules that only allow filesystem read actions
>>> +and establish a specific TCP connection, but filesystem write actions
>>> +and other TCP actions will be denied.  The ruleset then needs to handle both of
>>> +these kind of actions.  This is required for backward and forward compatibility
>>> +(i.e. the kernel and user space may not know each other's supported
>>> +restrictions), hence the need to be explicit about the denied-by-default access
>>> +rights.
>> 
>> I think it became a bit long - I'd suggest to split it into multiple
>> paragraphs, one after "our rules." (in line with landlock(7)), and one
>> after "will be denied."
>> 
>> Maybe the long sentence "For this example, ..." in the middle
>> paragraph could also be split up in two, to make it more readable?  I
>> think the point of that sentence is really just to give a brief
>> overview over what ruleset we are setting out to write.
>> 
>>>
>>>   .. code-block:: c
>>>
>>> @@ -62,6 +66,9 @@ the need to be explicit about the denied-by-default access rights.
>>>               LANDLOCK_ACCESS_FS_MAKE_SYM |
>>>               LANDLOCK_ACCESS_FS_REFER |
>>>               LANDLOCK_ACCESS_FS_TRUNCATE,
>>> +        .handled_access_net =
>>> +            LANDLOCK_ACCESS_NET_BIND_TCP |
>>> +            LANDLOCK_ACCESS_NET_CONNECT_TCP,
>>>       };
>>>
>>>   Because we may not know on which kernel version an application will be
>>> @@ -70,14 +77,18 @@ should try to protect users as much as possible whatever the kernel they are
>>>   using.  To avoid binary enforcement (i.e. either all security features or
>>>   none), we can leverage a dedicated Landlock command to get the current version
>>>   of the Landlock ABI and adapt the handled accesses.  Let's check if we should
>>> -remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
>>> -access rights, which are only supported starting with the second and third
>>> -version of the ABI.
>>> +remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE`` or
>>> +network access rights, which are only supported starting with the second,
>>> +third and fourth version of the ABI.
>> 
>> At some point it becomes too much to spell it out in one sentence; I'd recommend
>> 
>>    Let's check if we should remove access rights which are only supported
>>    in higher versions of the ABI.
>> 
>>>
>>>   .. code-block:: c
>>>
>>>       int abi;
>>>
>>> +    #define ACCESS_NET_BIND_CONNECT ( \
>>> +        LANDLOCK_ACCESS_NET_BIND_TCP | \
>>> +        LANDLOCK_ACCESS_NET_CONNECT_TCP)
>>> +
>> 
>> This #define does not seem to be used? -- Drop it?
>> 
>> 
>>>       abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
>>>       if (abi < 0) {
>>>           /* Degrades gracefully if Landlock is not handled. */
>>> @@ -92,6 +103,11 @@ version of the ABI.
>>>       case 2:
>>>           /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
>>>           ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
>>> +    case 3:
>>> +        /* Removes network support for ABI < 4 */
>>> +        ruleset_attr.handled_access_net &=
>>> +            ~(LANDLOCK_ACCESS_NET_BIND_TCP |
>>> +              LANDLOCK_ACCESS_NET_CONNECT_TCP);
>>>       }
>>>
>>>   This enables to create an inclusive ruleset that will contain our rules.
>>> @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
>>>   ABI version.  In this example, this is not required because all of the requested
>>>   ``allowed_access`` rights are already available in ABI 1.
>>>
>>> -We now have a ruleset with one rule allowing read access to ``/usr`` while
>>> -denying all other handled accesses for the filesystem.  The next step is to
>>> -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
>>> -binary).
>>> +For network access-control, we can add a set of rules that allow to use a port
>>> +number for a specific action: HTTPS connections.
>>> +
>>> +.. code-block:: c
>>> +
>>> +    struct landlock_net_service_attr net_service = {
>>> +        .allowed_access = NET_CONNECT_TCP,
>>> +        .port = 443,
>>> +    };
>>> +
>>> +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>>> +                            &net_service, 0);
>>> +
>>> +The next step is to restrict the current thread from gaining more privileges
>>> +(e.g. through a SUID binary). We now have a ruleset with the first rule allowing
>>> +read access to ``/usr`` while denying all other handled accesses for the filesystem,
>>> +and a second rule allowing HTTPS connections.
>>>
>>>   .. code-block:: c
>>>
>>> @@ -355,7 +384,7 @@ Access rights
>>>   -------------
>>>
>>>   .. kernel-doc:: include/uapi/linux/landlock.h
>>> -    :identifiers: fs_access
>>> +    :identifiers: fs_access net_access
>>>
>>>   Creating a new ruleset
>>>   ----------------------
>>> @@ -374,6 +403,7 @@ Extending a ruleset
>>>
>>>   .. kernel-doc:: include/uapi/linux/landlock.h
>>>       :identifiers: landlock_rule_type landlock_path_beneath_attr
>>> +                  landlock_net_service_attr
>>>
>>>   Enforcing a ruleset
>>>   -------------------
>>> @@ -451,6 +481,12 @@ always allowed when using a kernel that only supports the first or second ABI.
>>>   Starting with the Landlock ABI version 3, it is now possible to securely control
>>>   truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.
>>>
>>> +Network support (ABI < 4)
>>> +-------------------------
>>> +
>>> +Starting with the Landlock ABI version 4, it is now possible to restrict TCP
>>> +bind and connect actions to only a set of allowed ports.
>>> +
>>>   .. _kernel_support:
>>>
>>>   Kernel support
>>> @@ -469,6 +505,11 @@ still enable it by adding ``lsm=landlock,[...]`` to
>>>   Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
>>>   configuration.
>>>
>>> +To be able to explicitly allow TCP operations (e.g., adding a network rule with
>>> +``LANDLOCK_ACCESS_NET_TCP_BIND``), the kernel must support TCP (``CONFIG_INET=y``).
>>> +Otherwise, sys_landlock_add_rule() returns an ``EAFNOSUPPORT`` error, which can
>>> +safely be ignored because this kind of TCP operation is already not possible.
>>> +
>>>   Questions and answers
>>>   =====================
>>>
>>> --
>>> 2.25.1
>>>
>> 
>> —Günther
>> 
> .

Mickaël Salaün June 22, 2023, 4:50 p.m. UTC | #7

On 13/06/2023 22:12, Mickaël Salaün wrote:
> 
> 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 <[e-mail]> 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 <[e-mail]>

[...]

>>>>> @@ -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?

Well, here is a more accurate and generic definition (using "scope"):

A Landlock rule describes a set of actions intended by a task on a scope 
of objects.  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 scope of objects is a file hierarchy,
     and the related filesystem actions are defined with
     `filesystem access rights`.

Network rules (since ABI v4)
     For these rules, the scope of objects is the sockets identified
     with a TCP (bind or connect) port according to the related
     `network access rights`.


What do you think?


>>>
>>>>         and the related actions are defined with `network access rights`.

Jeff Xu June 23, 2023, 2:35 p.m. UTC | #8

On Thu, Jun 22, 2023 at 9:50 AM Mickaël Salaün <[e-mail]> wrote:
>
>
> On 13/06/2023 22:12, Mickaël Salaün wrote:
> >
> > 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 <[e-mail]> 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 <[e-mail]>
>
> [...]
>
> >>>>> @@ -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?
>
> Well, here is a more accurate and generic definition (using "scope"):
>
> A Landlock rule describes a set of actions intended by a task on a scope
> of objects.  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 scope of objects is a file hierarchy,
>      and the related filesystem actions are defined with
>      `filesystem access rights`.
>
> Network rules (since ABI v4)
>      For these rules, the scope of objects is the sockets identified
>      with a TCP (bind or connect) port according to the related
>      `network access rights`.
>
>
> What do you think?
>
I found this is clearer to me (mention of bind/connect port).

In networking, "5-tuple" is a well-known term for connection, which is
src/dest ip, src/dest port, protocol. That is why I asked about
src/dest port.  It seems that we only support src/dest port at this
moment, right ?

Another feature we could consider is restricting a process to "no
network access, allow out-going , allow incoming", this might overlap
with seccomp, but I think it is convenient to have it in Landlock.

Adding protocol restriction is a low hanging fruit also, for example,
a process might be restricted to UDP only (for RTP packet), and
another process for TCP (for signaling) , etc.

Thanks!
-Jeff Xu

>
> >>>
> >>>>         and the related actions are defined with `network access rights`.

Mickaël Salaün June 26, 2023, 6:59 p.m. UTC | #9

On 15/05/2023 18:13, 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 <[e-mail]>
> ---
> 
> Changes since v10:
> * Fixes documentaion as Mickaёl suggested:
> https://lore.kernel.org/linux-security-module/[e-mail]/
> 
> 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/[e-mail]/
> 
> Changes since v6:
> * Adds network support documentaion.
> 
> ---
>   Documentation/userspace-api/landlock.rst | 83 ++++++++++++++++++------
>   1 file changed, 62 insertions(+), 21 deletions(-)
> 

[...]

> @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
>   ABI version.  In this example, this is not required because all of the requested
>   ``allowed_access`` rights are already available in ABI 1.
> 
> -We now have a ruleset with one rule allowing read access to ``/usr`` while
> -denying all other handled accesses for the filesystem.  The next step is to
> -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
> -binary).
> +For network access-control, we can add a set of rules that allow to use a port
> +number for a specific action: HTTPS connections.
> +
> +.. code-block:: c
> +
> +    struct landlock_net_service_attr net_service = {
> +        .allowed_access = NET_CONNECT_TCP,

LANDLOCK_ACCESS_NET_CONNECT_TCP


> +        .port = 443,
> +    };
> +
> +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +                            &net_service, 0);
> +

Konstantin Meskhidze (A) July 3, 2023, 9:04 a.m. UTC | #10

6/23/2023 5:35 PM, Jeff Xu пишет:
> On Thu, Jun 22, 2023 at 9:50 AM Mickaël Salaün <[e-mail]> wrote:
>>
>>
>> On 13/06/2023 22:12, Mickaël Salaün wrote:
>> >
>> > 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 <[e-mail]> 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 <[e-mail]>
>>
>> [...]
>>
>> >>>>> @@ -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?
>>
>> Well, here is a more accurate and generic definition (using "scope"):
>>
>> A Landlock rule describes a set of actions intended by a task on a scope
>> of objects.  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 scope of objects is a file hierarchy,
>>      and the related filesystem actions are defined with
>>      `filesystem access rights`.
>>
>> Network rules (since ABI v4)
>>      For these rules, the scope of objects is the sockets identified
>>      with a TCP (bind or connect) port according to the related
>>      `network access rights`.
>>
>>
>> What do you think?
>>
> I found this is clearer to me (mention of bind/connect port).
> 
> In networking, "5-tuple" is a well-known term for connection, which is
> src/dest ip, src/dest port, protocol. That is why I asked about
> src/dest port.  It seems that we only support src/dest port at this
> moment, right ?
> 
> Another feature we could consider is restricting a process to "no
> network access, allow out-going , allow incoming", this might overlap
> with seccomp, but I think it is convenient to have it in Landlock.
> 
> Adding protocol restriction is a low hanging fruit also, for example,
> a process might be restricted to UDP only (for RTP packet), and
> another process for TCP (for signaling) , etc.

  Hi,
   By the way, UPD protocol brings more performance challenges here 
beacuse it does not establish a connection so every UDP packet will be 
hooked by Landlock to check apllied rules.
> 
> Thanks!
> -Jeff Xu
> 
>>
>> >>>
>> >>>>         and the related actions are defined with `network access rights`.
> .

Konstantin Meskhidze (A) July 3, 2023, 10:42 a.m. UTC | #11

6/26/2023 9:59 PM, Mickaël Salaün пишет:
> 
> On 15/05/2023 18:13, 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 <[e-mail]>
>> ---
>> 
>> Changes since v10:
>> * Fixes documentaion as Mickaёl suggested:
>> https://lore.kernel.org/linux-security-module/[e-mail]/
>> 
>> 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/[e-mail]/
>> 
>> Changes since v6:
>> * Adds network support documentaion.
>> 
>> ---
>>   Documentation/userspace-api/landlock.rst | 83 ++++++++++++++++++------
>>   1 file changed, 62 insertions(+), 21 deletions(-)
>> 
> 
> [...]
> 
>> @@ -143,10 +159,23 @@ for the ruleset creation, by filtering access rights according to the Landlock
>>   ABI version.  In this example, this is not required because all of the requested
>>   ``allowed_access`` rights are already available in ABI 1.
>> 
>> -We now have a ruleset with one rule allowing read access to ``/usr`` while
>> -denying all other handled accesses for the filesystem.  The next step is to
>> -restrict the current thread from gaining more privileges (e.g. thanks to a SUID
>> -binary).
>> +For network access-control, we can add a set of rules that allow to use a port
>> +number for a specific action: HTTPS connections.
>> +
>> +.. code-block:: c
>> +
>> +    struct landlock_net_service_attr net_service = {
>> +        .allowed_access = NET_CONNECT_TCP,
> 
> LANDLOCK_ACCESS_NET_CONNECT_TCP

   Yep. Thanks.
> 
> 
>> +        .port = 443,
>> +    };
>> +
>> +    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> +                            &net_service, 0);
>> +
> .

Mickaël Salaün July 3, 2023, 5:04 p.m. UTC | #12

On 03/07/2023 11:04, Konstantin Meskhidze (A) wrote:
> 
> 
> 6/23/2023 5:35 PM, Jeff Xu пишет:
>> On Thu, Jun 22, 2023 at 9:50 AM Mickaël Salaün <[e-mail]> wrote:
>>>
>>>
>>> On 13/06/2023 22:12, Mickaël Salaün wrote:
>>>>
>>>> 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 <[e-mail]> 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 <[e-mail]>
>>>
>>> [...]
>>>
>>>>>>>> @@ -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?
>>>
>>> Well, here is a more accurate and generic definition (using "scope"):
>>>
>>> A Landlock rule describes a set of actions intended by a task on a scope
>>> of objects.  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 scope of objects is a file hierarchy,
>>>       and the related filesystem actions are defined with
>>>       `filesystem access rights`.
>>>
>>> Network rules (since ABI v4)
>>>       For these rules, the scope of objects is the sockets identified
>>>       with a TCP (bind or connect) port according to the related
>>>       `network access rights`.
>>>
>>>
>>> What do you think?
>>>
>> I found this is clearer to me (mention of bind/connect port).
>>
>> In networking, "5-tuple" is a well-known term for connection, which is
>> src/dest ip, src/dest port, protocol. That is why I asked about
>> src/dest port.  It seems that we only support src/dest port at this
>> moment, right ?
>>
>> Another feature we could consider is restricting a process to "no
>> network access, allow out-going , allow incoming", this might overlap
>> with seccomp, but I think it is convenient to have it in Landlock.
>>
>> Adding protocol restriction is a low hanging fruit also, for example,
>> a process might be restricted to UDP only (for RTP packet), and
>> another process for TCP (for signaling) , etc.
> 
>    Hi,
>     By the way, UPD protocol brings more performance challenges here
> beacuse it does not establish a connection so every UDP packet will be
> hooked by Landlock to check apllied rules.

Correct, but I think that for performant-sensitive applications, it 
would not be an issue to check sendto and recvfrom for UDP because they 
should use connect/bind and send/recv instead, and we can check 
connect/bind without checking send/recv. Indeed, bind and connect can be 
used to configure an UDP socket, even if it is not a connected protocol 
this enables to limit the number of kernel checks and copies. I'm not 
sure how many applications use sendto and recvfrom though.

For thread dedicated to UDP, see 
https://lore.kernel.org/r/[e-mail]

Patch

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.

 Defining and enforcing a security policy
 ----------------------------------------

 We first need to define the ruleset that will contain our rules.  For this
-example, the ruleset will contain rules that only allow read actions, but write
-actions will be denied.  The ruleset then needs to handle both of these kind of
-actions.  This is required for backward and forward compatibility (i.e. the
-kernel and user space may not know each other's supported restrictions), hence
-the need to be explicit about the denied-by-default access rights.
+example, the ruleset will contain rules that only allow filesystem read actions
+and establish a specific TCP connection, but filesystem write actions
+and other TCP actions will be denied.  The ruleset then needs to handle both of
+these kind of actions.  This is required for backward and forward compatibility
+(i.e. the kernel and user space may not know each other's supported
+restrictions), hence the need to be explicit about the denied-by-default access
+rights.

 .. code-block:: c

@@ -62,6 +66,9 @@  the need to be explicit about the denied-by-default access rights.
             LANDLOCK_ACCESS_FS_MAKE_SYM |
             LANDLOCK_ACCESS_FS_REFER |
             LANDLOCK_ACCESS_FS_TRUNCATE,
+        .handled_access_net =
+            LANDLOCK_ACCESS_NET_BIND_TCP |
+            LANDLOCK_ACCESS_NET_CONNECT_TCP,
     };

 Because we may not know on which kernel version an application will be
@@ -70,14 +77,18 @@  should try to protect users as much as possible whatever the kernel they are
 using.  To avoid binary enforcement (i.e. either all security features or
 none), we can leverage a dedicated Landlock command to get the current version
 of the Landlock ABI and adapt the handled accesses.  Let's check if we should
-remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
-access rights, which are only supported starting with the second and third
-version of the ABI.
+remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE`` or
+network access rights, which are only supported starting with the second,
+third and fourth version of the ABI.

 .. code-block:: c

     int abi;

+    #define ACCESS_NET_BIND_CONNECT ( \
+        LANDLOCK_ACCESS_NET_BIND_TCP | \
+        LANDLOCK_ACCESS_NET_CONNECT_TCP)
+
     abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
     if (abi < 0) {
         /* Degrades gracefully if Landlock is not handled. */
@@ -92,6 +103,11 @@  version of the ABI.
     case 2:
         /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
         ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
+    case 3:
+        /* Removes network support for ABI < 4 */
+        ruleset_attr.handled_access_net &=
+            ~(LANDLOCK_ACCESS_NET_BIND_TCP |
+              LANDLOCK_ACCESS_NET_CONNECT_TCP);
     }

 This enables to create an inclusive ruleset that will contain our rules.
@@ -143,10 +159,23 @@  for the ruleset creation, by filtering access rights according to the Landlock
 ABI version.  In this example, this is not required because all of the requested
 ``allowed_access`` rights are already available in ABI 1.

-We now have a ruleset with one rule allowing read access to ``/usr`` while
-denying all other handled accesses for the filesystem.  The next step is to
-restrict the current thread from gaining more privileges (e.g. thanks to a SUID
-binary).
+For network access-control, we can add a set of rules that allow to use a port
+number for a specific action: HTTPS connections.
+
+.. code-block:: c
+
+    struct landlock_net_service_attr net_service = {
+        .allowed_access = NET_CONNECT_TCP,
+        .port = 443,
+    };
+
+    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
+                            &net_service, 0);
+
+The next step is to restrict the current thread from gaining more privileges
+(e.g. through a SUID binary). We now have a ruleset with the first rule allowing
+read access to ``/usr`` while denying all other handled accesses for the filesystem,
+and a second rule allowing HTTPS connections.

 .. code-block:: c

@@ -355,7 +384,7 @@  Access rights
 -------------

 .. kernel-doc:: include/uapi/linux/landlock.h
-    :identifiers: fs_access
+    :identifiers: fs_access net_access

 Creating a new ruleset
 ----------------------
@@ -374,6 +403,7 @@  Extending a ruleset

 .. kernel-doc:: include/uapi/linux/landlock.h
     :identifiers: landlock_rule_type landlock_path_beneath_attr
+                  landlock_net_service_attr

 Enforcing a ruleset
 -------------------
@@ -451,6 +481,12 @@  always allowed when using a kernel that only supports the first or second ABI.
 Starting with the Landlock ABI version 3, it is now possible to securely control
 truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.

+Network support (ABI < 4)
+-------------------------
+
+Starting with the Landlock ABI version 4, it is now possible to restrict TCP
+bind and connect actions to only a set of allowed ports.
+
 .. _kernel_support:

 Kernel support
@@ -469,6 +505,11 @@  still enable it by adding ``lsm=landlock,[...]`` to
 Documentation/admin-guide/kernel-parameters.rst thanks to the bootloader
 configuration.

+To be able to explicitly allow TCP operations (e.g., adding a network rule with
+``LANDLOCK_ACCESS_NET_TCP_BIND``), the kernel must support TCP (``CONFIG_INET=y``).
+Otherwise, sys_landlock_add_rule() returns an ``EAFNOSUPPORT`` error, which can
+safely be ignored because this kind of TCP operation is already not possible.
+
 Questions and answers
 =====================