Re: [PATCH v3 1/2] landlock.7, landlock_*.2: Document Landlock ABI version 4

["Günther Noack" <gnoack@google.com>]

Hello Alejandro!

On Tue, Jul 23, 2024 at 03:03:13PM +0200, Alejandro Colomar wrote:
> On Tue, Jul 23, 2024 at 10:19:16AM GMT, Günther Noack wrote:
> > +.EX
> > +struct landlock_net_port_attr {
> > +    __u64 allowed_access;
> > +    __u64 port;
> > +};
> > +.EE
> > +.in
> > +.IP
> > +.i allowed_access
> 
> s/i/I/

Thanks, fixed. o_O


> > +contains a bitmask of allowed network actions,
> > +which can be applied on the given port.
> > +.IP
> > +.i port

Same here


> > +is the network port in host endianness.
> > +.IP
> > +It should be noted that port 0 passed to
> > +.BR bind (2)
> > +will bind to an available port from the ephemeral port range.
> > +This can be configured in the
> > +.I /proc/sys/net/ipv4/ip_local_port_range
> > +sysctl (also used for IPv6).
> > +.IP
> > +A Landlock rule with port 0
> > +and the
> > +.B LANDLOCK_ACCESS_NET_BIND_TCP
> > +right means that requesting to bind on port 0 is allowed
> > +and it will automatically translate to binding on the related port range.
> >  .P
> >  .I flags
> >  must be 0.
> > @@ -89,6 +132,12 @@ is set to indicate the error.
> >  .BR landlock_add_rule ()
> >  can fail for the following reasons:
> >  .TP
> > +.B EAFNOSUPPORT
> > +.I rule_type
> > +is
> > +.BR LANDLOCK_RULE_NET_PORT ,
> > +but TCP is not supported by the running kernel.
> > +.TP
> >  .B EOPNOTSUPP
> >  Landlock is supported by the kernel but disabled at boot time.
> >  .TP
> > @@ -111,6 +160,11 @@ are only applicable to directories, but
> >  .I \%rule_attr\->parent_fd
> >  does not refer to a directory).
> >  .TP
> > +.B EINVAL
> > +In
> > +.IR \%struct\~landlock_net_port_attr ,
> > +the port number is greater than 65535.
> > +.TP
> >  .B ENOMSG
> >  Empty accesses (i.e.,
> >  .I rule_attr\->allowed_access
> > diff --git a/man/man2/landlock_create_ruleset.2 b/man/man2/landlock_create_ruleset.2
> > index 105e9b062..ca635ddbc 100644
> > --- a/man/man2/landlock_create_ruleset.2
> > +++ b/man/man2/landlock_create_ruleset.2
> > @@ -41,6 +41,7 @@ It points to the following structure:
> >  .EX
> >  struct landlock_ruleset_attr {
> >      __u64 handled_access_fs;
> > +    __u64 handled_access_net;
> >  };
> >  .EE
> >  .in
> > @@ -52,6 +53,13 @@ is a bitmask of handled filesystem actions
> >  in
> >  .BR landlock (7)).
> >  .IP
> > +.I handled_access_net
> > +is a bitmask of handled network actions
> > +(see
> > +.B Network actions
> > +in
> > +.BR landlock (7)).
> > +.IP
> >  This structure defines a set of
> >  .IR "handled access rights" ,
> >  a set of actions on different object types,
> > @@ -143,8 +151,8 @@ was not a valid address.
> >  .TP
> >  .B ENOMSG
> >  Empty accesses (i.e.,
> > -.I attr\->handled_access_fs
> > -is 0).
> > +.I attr
> > +did not specify any access rights to restrict).
> 
> This looks like a wording fix, isn't it?  If so, it might be worth a
> separate patch.
> 
> >  .SH STANDARDS
> >  Linux.
> >  .SH HISTORY
> > diff --git a/man/man7/landlock.7 b/man/man7/landlock.7
> > index 652054f15..52876a3de 100644
> > --- a/man/man7/landlock.7
> > +++ b/man/man7/landlock.7
> > @@ -189,6 +189,19 @@ If multiple requirements are not met, the
> >  error code takes precedence over
> >  .BR EXDEV .
> >  .\"
> > +.SS Network flags
> > +These flags enable to restrict a sandboxed process
> > +to a set of network actions.
> > +This is supported since the Landlock ABI version 4.
> > +.P
> > +The following access rights apply to TCP port numbers:
> > +.TP
> > +.B LANDLOCK_ACCESS_NET_BIND_TCP
> > +Bind a TCP socket to a local port.
> > +.TP
> > +.B LANDLOCK_ACCESS_NET_CONNECT_TCP
> > +Connect an active TCP socket to a remote port.
> > +.\"
> >  .SS Layers of file path access rights
> >  Each time a thread enforces a ruleset on itself,
> >  it updates its Landlock domain with a new layer of policy.
> > @@ -339,6 +352,9 @@ _	_	_
> >  2	5.19	LANDLOCK_ACCESS_FS_REFER
> >  _	_	_
> >  3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
> > +_	_	_
> > +4	6.7	LANDLOCK_ACCESS_NET_BIND_TCP
> > +\^	\^	LANDLOCK_ACCESS_NET_CONNECT_TCP
> 
> Did you actually want \[ha]?

I believe \[ha] would show the caret character in the document?
What I want is different, as documented in tbl(1):

  On any row but the first, a table entry of \^
  causes the entry above it to span down into the current one.

The table cell should render like this:

  +---+-----+---------------------------------+
  | 4 | 6.7 | LANDLOCK_ACCESS_NET_BIND_TCP    |
  |   |     | LANDLOCK_ACCESS_NET_CONNECT_TCP |
  +---+-----+---------------------------------+

We are already using the same approach for the table cells further up in the
table.  IIRC, the tbl(1) preprocessor wanted *something* there, and \^ is the
placeholder that you can put if you don't have text to put in that place?

Assuming that I'm right about the \^, do you want a separate patch set for the
.i/.I change?

—Günther