All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: Tingmao Wang <m@maowtm.org>
Cc: "Günther Noack" <gnoack@google.com>,
	"Demi Marie Obenour" <demiobenour@gmail.com>,
	"Alyssa Ross" <hi@alyssa.is>, "Jann Horn" <jannh@google.com>,
	"Tahera Fahimi" <fahimitahera@gmail.com>,
	linux-security-module@vger.kernel.org,
	"Justin Suess" <utilityemal77@gmail.com>
Subject: Re: [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
Date: Sun, 28 Dec 2025 19:15:45 +0100	[thread overview]
Message-ID: <20251228.aeX5Aighashi@digikod.net> (raw)
In-Reply-To: <be315d8ff7544fd91bdb922e8afc7c8154e3594d.1766925301.git.m@maowtm.org>

On Sun, Dec 28, 2025 at 12:45:41PM +0000, Tingmao Wang wrote:
> Extend the existing abstract UNIX socket scoping to pathname sockets as
> well.  Basically all of the logic is reused between the two types, just
> that pathname sockets scoping are controlled by another bit, and has its
> own audit request type (since the current one is named
> "abstract_unix_socket").
> 
> Closes: https://github.com/landlock-lsm/linux/issues/51
> Signed-off-by: Tingmao Wang <m@maowtm.org>

Great, thanks!

> ---
> 
> There is an argument that there should only really be one audit request
> type for both sockets, since the only difference is whether path= is
> followed by a normal path, or by a hex string starting with 00.  But I'm
> not sure if we can change this at this point, so I have created a new
> request type.

It is the correct approach to add a dedicated request type, which
enables to filter on it, and doesn't have performance impact.

> 
>  security/landlock/audit.c |  4 +++
>  security/landlock/audit.h |  1 +
>  security/landlock/task.c  | 74 ++++++++++++++++++++++++++++++---------
>  3 files changed, 62 insertions(+), 17 deletions(-)

> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 6dfcc1860d6e..9fbb0ada440b 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c

> +/*
> + * UNIX sockets can have three types of addresses: pathname (a filesystem path),
> + * unnamed (not bound to an address), and abstract (sun_path[0] is '\0').
> + * Unnamed sockets include those created with socketpair() and unbound sockets.
> + * We do not restrict unnamed sockets since they have no address to identify.
> + */
>  static int hook_unix_stream_connect(struct sock *const sock,
>  				    struct sock *const other,
>  				    struct sock *const newsk)
>  {
>  	size_t handle_layer;
> +	access_mask_t scope;
> +	enum landlock_request_type request_type;
>  	const struct landlock_cred_security *const subject =
>  		landlock_get_applicable_subject(current_cred(), unix_scope,
>  						&handle_layer);
> +	const struct unix_address *addr;
>  
>  	/* Quick return for non-landlocked tasks. */
>  	if (!subject)
>  		return 0;
>  


> -	if (!is_abstract_socket(other))
> +	addr = unix_sk(other)->addr;
> +	/* Unnamed sockets are not restricted. */
> +	if (!addr)
>  		return 0;
>  
> -	if (!sock_is_scoped(other, subject->domain))
> +	if (sock_addr_is_abstract(addr)) {
> +		scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
> +		request_type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET;
> +	} else {
> +		/* Pathname socket. */
> +		scope = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> +		request_type = LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET;
> +	}
> +
> +	if (!sock_is_scoped(other, subject->domain, scope))
>  		return 0;

We should be able to factor out this hunk for both hooks, and then also
fold is_abstract_socket() in this new helper.

>  
>  	landlock_log_denial(subject, &(struct landlock_request) {
> -		.type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> +		.type = request_type,
>  		.audit = {
>  			.type = LSM_AUDIT_DATA_NET,
>  			.u.net = &(struct lsm_network_audit) {
> @@ -299,9 +326,12 @@ static int hook_unix_may_send(struct socket *const sock,
>  			      struct socket *const other)
>  {
>  	size_t handle_layer;
> +	access_mask_t scope;
> +	enum landlock_request_type request_type;
>  	const struct landlock_cred_security *const subject =
>  		landlock_get_applicable_subject(current_cred(), unix_scope,
>  						&handle_layer);
> +	const struct unix_address *addr;
>  
>  	if (!subject)
>  		return 0;
> @@ -313,14 +343,24 @@ static int hook_unix_may_send(struct socket *const sock,
>  	if (unix_peer(sock->sk) == other->sk)
>  		return 0;
>  
> -	if (!is_abstract_socket(other->sk))
> +	addr = unix_sk(other->sk)->addr;
> +	/* Unnamed sockets are not restricted. */
> +	if (!addr)
>  		return 0;
>  
> -	if (!sock_is_scoped(other->sk, subject->domain))
> +	if (sock_addr_is_abstract(addr)) {
> +		scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET;
> +		request_type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET;
> +	} else {
> +		scope = LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET;
> +		request_type = LANDLOCK_REQUEST_SCOPE_PATHNAME_UNIX_SOCKET;
> +	}
> +
> +	if (!sock_is_scoped(other->sk, subject->domain, scope))
>  		return 0;
>  
>  	landlock_log_denial(subject, &(struct landlock_request) {
> -		.type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
> +		.type = request_type,
>  		.audit = {
>  			.type = LSM_AUDIT_DATA_NET,
>  			.u.net = &(struct lsm_network_audit) {
> -- 
> 2.52.0
> 

  parent reply	other threads:[~2025-12-28 18:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-28 12:45 [PATCH 0/6] Landlock: Implement scope control for pathname Unix sockets Tingmao Wang
2025-12-28 12:45 ` [PATCH 1/6] landlock: Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET scope bit to uAPI Tingmao Wang
2025-12-28 12:45 ` [PATCH 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET Tingmao Wang
2025-12-28 16:37   ` Justin Suess
2025-12-30 15:52     ` Tingmao Wang
2025-12-30 15:56       ` Tingmao Wang
2025-12-28 18:15   ` Mickaël Salaün [this message]
2025-12-28 12:45 ` [PATCH 3/6] samples/landlock: Support LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET Tingmao Wang
2025-12-29  2:48   ` Demi Marie Obenour
2025-12-28 12:45 ` [PATCH 4/6] selftests/landlock: Support pathname socket path in set_unix_address Tingmao Wang
2025-12-28 12:45 ` [PATCH 5/6] selftests/landlock: Repurpose scoped_abstract_unix_test.c for pathname sockets too Tingmao Wang
2025-12-28 12:45 ` [PATCH 6/6] selftests/landlock: Add pathname socket variants for more tests Tingmao Wang

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=20251228.aeX5Aighashi@digikod.net \
    --to=mic@digikod.net \
    --cc=demiobenour@gmail.com \
    --cc=fahimitahera@gmail.com \
    --cc=gnoack@google.com \
    --cc=hi@alyssa.is \
    --cc=jannh@google.com \
    --cc=linux-security-module@vger.kernel.org \
    --cc=m@maowtm.org \
    --cc=utilityemal77@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.