All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Günther Noack" <gnoack@google.com>
To: Alejandro Colomar <alx@kernel.org>
Cc: "Mickaël Salaün" <mic@digikod.net>,
	"Tahera Fahimi" <fahimitahera@gmail.com>,
	"Tanya Agarwal" <tanyaagarwal25699@gmail.com>,
	linux-security-module@vger.kernel.org, linux-man@vger.kernel.org,
	"Daniel Burgener" <dburgener@linux.microsoft.com>
Subject: Re: [PATCH v2 2/3] landlock.7: Move over documentation for ABI version 6
Date: Mon, 3 Mar 2025 17:24:45 +0100	[thread overview]
Message-ID: <Z8XXzUggsHkRLEqG@google.com> (raw)
In-Reply-To: <sbib2esl6bev7tqww3rgyykpxorpyaix7dujwwm2pg42egg6an@rdyjnecj5vti>

Hello Alejandro!

For context, in this patch set, we have three commits:

  * 1/3 and 2/3 copy documentation from the kernel side unmodified.
  * 3/3 revises a section about Landlock's "scoped" restriction features.

I thought it would be easier to discuss with the "copy" and "rewrite" parts
separate, but actually, as you also noticed, 3/3 does rewrite large chunks of
the 2/3 commit along the way, and it is probably not worth correcting much of
that wording any more.

Would you prefer if I squashed commits 2/3 and 3/3 into one?

On Fri, Feb 28, 2025 at 10:23:47PM +0100, Alejandro Colomar wrote:
> On Wed, Feb 26, 2025 at 10:29:11PM +0100, Günther Noack wrote:
> > With this ABI version, Landlock can restrict outgoing interactions with
> > higher-privileged Landlock domains through Abstract Unix Domain sockets and
> > signals.
> > 
> > Signed-off-by: Günther Noack <gnoack@google.com>
> > ---
> >  man/man7/landlock.7 | 69 ++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 68 insertions(+), 1 deletion(-)
> > 
> > diff --git a/man/man7/landlock.7 b/man/man7/landlock.7
> > index 11f76b072..30dbac73d 100644
> > --- a/man/man7/landlock.7
> > +++ b/man/man7/landlock.7
> > @@ -248,7 +248,8 @@ This access right is available since the fifth version of the Landlock ABI.
> >  .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
> > +This is supported since Landlock ABI version 4.
> >  .P
> >  The following access rights apply to TCP port numbers:
> >  .TP
> > @@ -258,6 +259,24 @@ Bind a TCP socket to a local port.
> >  .B LANDLOCK_ACCESS_NET_CONNECT_TCP
> >  Connect an active TCP socket to a remote port.
> >  .\"
> > +.SS Scope flags
> > +These flags enable to isolate a sandboxed process from a set of IPC actions.
> 
> s/to isolate/isolating/
> 
> AFAIU, to be able to use an infinitive with enable/allow you need a
> direct object in the sentence.  If there's no direct object, you need a
> gerund.

Thanks, this is useful.  Changed it to infinitive for now.

FWIW, the same phrases exist on the kernel side as well, unfortunately.

> > +Setting a flag for a ruleset will isolate the Landlock domain
> > +to forbid connections to resources outside the domain.
> > +.P
> > +This is supported since Landlock ABI version 6.
> 
> I'm wondering if we should have this as a parenthetical next to the
> title, like we usually do with "(since Linux X.Y)".  Don't do it for
> now, but please consider it for when you have some time.  I'm not saying
> you should do it though, just that you consider it, and tell me if you
> agree or not.

I added it to my notes for further revisions,
I think this would indeed be more appropriate in the man pages.

Is it possible to set the paranthetical without bold as well,
even in a .SS subsection header?


> > +.P
> > +The following scopes exist:
> > +.TP
> > +.B LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET
> > +Restrict a sandboxed process from connecting to an abstract UNIX socket
> > +created by a process outside the related Landlock domain
> > +(e.g., a parent domain or a non-sandboxed process).
> > +.TP
> > +.B LANDLOCK_SCOPE_SIGNAL
> > +Restrict a sandboxed process from sending a signal
> > +to another process outside the domain.
> > +.\"
> >  .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.
> > @@ -334,6 +353,51 @@ and related syscalls on a target process,
> >  a sandboxed process should have a subset of the target process rules,
> >  which means the tracee must be in a sub-domain of the tracer.
> >  .\"
> > +.SS IPC scoping
> > +Similar to the implicit
> > +.BR "Ptrace restrictions" ,
> > +we may want to further restrict interactions between sandboxes.
> > +Each Landlock domain can be explicitly scoped for a set of actions
> > +by specifying it on a ruleset.
> > +For example, if a sandboxed process should not be able to
> > +.BR connect (2)
> > +to a non-sandboxed process through abstract
> > +.BR unix (7)
> > +sockets,
> > +we can specify such a restriction with
> > +.BR LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET .
> > +Moreover, if a sandboxed process should not be able
> > +to send a signal to a non-sandboxed process,
> > +we can specify this restriction with
> > +.BR LANDLOCK_SCOPE_SIGNAL .
> > +.P
> > +A sandboxed process can connect to a non-sandboxed process
> > +when its domain is not scoped.
> 
> Does "its" refer to the sandboxed one or to the non-snadboxed one?

It refers to the sandboxed process.

This correction would be overwritten in the following commit.
I don't think it's worth fixing any more.

> > +If a process's domain is scoped,
> > +it can only connect to sockets created by processes in the same scope.
> > +Moreover,
> > +If a process is scoped to send signal
> 
> Is this a typo?  s/signal/&s/

It is a typo, copied from kernel documentation.  Oops.

This correction is overwritten in the following commit.


> > to a non-scoped process,
> 
> Should we use plural here?

This correction is overwritten in the following commit.

> > +it can only send signals to processes in the same scope.
> > +.P
> > +A connected datagram socket behaves like a stream socket
> > +when its domain is scoped,
> > +meaning if the domain is scoped after the socket is connected,
> > +it can still
> > +.BR send (2)
> > +data just like a stream socket.
> > +However, in the same scenario,
> > +a non-connected datagram socket cannot send data (with
> > +.BR sendto (2))
> > +outside its scope.
> > +.P
> > +A process with a scoped domain can inherit a socket
> > +created by a non-scoped process.
> > +The process cannot connect to this socket since it has a scoped domain.
> > +.P
> > +IPC scoping does not support exceptions, so if a domain is scoped,
> 
> Please break after the first ',' too.

Done.


> > +no rules can be added to allow access to resources or processes
> 
> Please break after the second 'to'.

Done.


> > +outside of the scope.

Thanks for the review,
—Günther

  reply	other threads:[~2025-03-03 16:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-26 21:18 [PATCH v2 0/1] landlock: Clarify IPC scoping documentation Günther Noack
2025-02-26 21:18 ` [PATCH v2 1/1] " Günther Noack
2025-02-26 21:29 ` [PATCH v2 1/3] landlock.7: Update description of Landlock rules Günther Noack
2025-02-26 21:40   ` Günther Noack
2025-02-28 21:31   ` Alejandro Colomar
2025-03-03 15:16     ` Günther Noack
2025-02-26 21:29 ` [PATCH v2 2/3] landlock.7: Move over documentation for ABI version 6 Günther Noack
2025-02-28 21:23   ` Alejandro Colomar
2025-03-03 16:24     ` Günther Noack [this message]
2025-03-03 18:30       ` Alejandro Colomar
2025-02-26 21:29 ` [PATCH v2 3/3] landlock.7: Clarify IPC scoping documentation in line with kernel side Günther Noack
2025-02-28 21:37   ` Alejandro Colomar
2025-03-03 16:36     ` Günther Noack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z8XXzUggsHkRLEqG@google.com \
    --to=gnoack@google.com \
    --cc=alx@kernel.org \
    --cc=dburgener@linux.microsoft.com \
    --cc=fahimitahera@gmail.com \
    --cc=linux-man@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --cc=tanyaagarwal25699@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.