diff mbox series

[v3,18/23] landlock: Log scoped denials

Message ID 20241122143353.59367-19-mic@digikod.net (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series Landlock audit support | expand

Commit Message

Mickaël Salaün Nov. 22, 2024, 2:33 p.m. UTC
Add audit support for unix_stream_connect, unix_may_send, task_kill, and
file_send_sigiotask hooks.

Audit event sample:

  type=LL_DENY [...]: domain=195ba459b blockers=scope_abstract_unix_socket path=00666F6F

Cc: Günther Noack <gnoack@google.com>
Cc: Tahera Fahimi <fahimitahera@gmail.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241122143353.59367-19-mic@digikod.net
---

Changes since v1:
- New patch.
---
 security/landlock/audit.c |  8 ++++++
 security/landlock/audit.h |  2 ++
 security/landlock/task.c  | 58 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 64 insertions(+), 4 deletions(-)

Comments

Paul Moore Jan. 5, 2025, 1:23 a.m. UTC | #1
On Nov 22, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> 
> Add audit support for unix_stream_connect, unix_may_send, task_kill, and
> file_send_sigiotask hooks.
> 
> Audit event sample:
> 
>   type=LL_DENY [...]: domain=195ba459b blockers=scope_abstract_unix_socket path=00666F6F

Similar to 17/23, I believe the SOCKADDR record should already capture
the socket address information.

It would also be nice to see an example record on a signal event.

> Cc: Günther Noack <gnoack@google.com>
> Cc: Tahera Fahimi <fahimitahera@gmail.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241122143353.59367-19-mic@digikod.net
> ---
> Changes since v1:
> - New patch.
> ---
>  security/landlock/audit.c |  8 ++++++
>  security/landlock/audit.h |  2 ++
>  security/landlock/task.c  | 58 ++++++++++++++++++++++++++++++++++++---
>  3 files changed, 64 insertions(+), 4 deletions(-)

--
paul-moore.com
Mickaël Salaün Jan. 6, 2025, 2:51 p.m. UTC | #2
On Sat, Jan 04, 2025 at 08:23:53PM -0500, Paul Moore wrote:
> On Nov 22, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> > 
> > Add audit support for unix_stream_connect, unix_may_send, task_kill, and
> > file_send_sigiotask hooks.
> > 
> > Audit event sample:
> > 
> >   type=LL_DENY [...]: domain=195ba459b blockers=scope_abstract_unix_socket path=00666F6F
> 
> Similar to 17/23, I believe the SOCKADDR record should already capture
> the socket address information.

This might not be the case, which is why SELinux and others explicitly
log it I guess.

> 
> It would also be nice to see an example record on a signal event.

Yes, I'll add such example.

> 
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Tahera Fahimi <fahimitahera@gmail.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > Link: https://lore.kernel.org/r/20241122143353.59367-19-mic@digikod.net
> > ---
> > Changes since v1:
> > - New patch.
> > ---
> >  security/landlock/audit.c |  8 ++++++
> >  security/landlock/audit.h |  2 ++
> >  security/landlock/task.c  | 58 ++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 64 insertions(+), 4 deletions(-)
> 
> --
> paul-moore.com
Paul Moore Jan. 6, 2025, 10:33 p.m. UTC | #3
On Mon, Jan 6, 2025 at 9:51 AM Mickaël Salaün <mic@digikod.net> wrote:
> On Sat, Jan 04, 2025 at 08:23:53PM -0500, Paul Moore wrote:
> > On Nov 22, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> > >
> > > Add audit support for unix_stream_connect, unix_may_send, task_kill, and
> > > file_send_sigiotask hooks.
> > >
> > > Audit event sample:
> > >
> > >   type=LL_DENY [...]: domain=195ba459b blockers=scope_abstract_unix_socket path=00666F6F
> >
> > Similar to 17/23, I believe the SOCKADDR record should already capture
> > the socket address information.
>
> This might not be the case, which is why SELinux and others explicitly
> log it I guess.

I think I may be misunderstanding you, can you point to the section of
SELinux code that you are referring to in your comment?
Mickaël Salaün Jan. 7, 2025, 2:23 p.m. UTC | #4
On Mon, Jan 06, 2025 at 05:33:07PM -0500, Paul Moore wrote:
> On Mon, Jan 6, 2025 at 9:51 AM Mickaël Salaün <mic@digikod.net> wrote:
> > On Sat, Jan 04, 2025 at 08:23:53PM -0500, Paul Moore wrote:
> > > On Nov 22, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> > > >
> > > > Add audit support for unix_stream_connect, unix_may_send, task_kill, and
> > > > file_send_sigiotask hooks.
> > > >
> > > > Audit event sample:
> > > >
> > > >   type=LL_DENY [...]: domain=195ba459b blockers=scope_abstract_unix_socket path=00666F6F
> > >
> > > Similar to 17/23, I believe the SOCKADDR record should already capture
> > > the socket address information.
> >
> > This might not be the case, which is why SELinux and others explicitly
> > log it I guess.
> 
> I think I may be misunderstanding you, can you point to the section of
> SELinux code that you are referring to in your comment?

I'm refering to struct lsm_network_audit, and the related information
ending in the logs with LSM_AUDIT_DATA_NET.  Landlock follows the
current implementations, hence the generalization brought by the second
patch with audit_log_lsm_data().
diff mbox series

Patch

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 0b69b4c9ba1c..7e4acc746cd6 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -71,6 +71,14 @@  get_blocker(const enum landlock_request_type type,
 		if (WARN_ON_ONCE(access_bit >= ARRAY_SIZE(net_access_strings)))
 			return "unknown";
 		return net_access_strings[access_bit];
+
+	case LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET:
+		WARN_ON_ONCE(access_bit != -1);
+		return "scope_abstract_unix_socket";
+
+	case LANDLOCK_REQUEST_SCOPE_SIGNAL:
+		WARN_ON_ONCE(access_bit != -1);
+		return "scope_signal";
 	}
 
 	WARN_ON_ONCE(1);
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 1075b0c8401f..1e0a9164bacc 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -19,6 +19,8 @@  enum landlock_request_type {
 	LANDLOCK_REQUEST_FS_CHANGE_LAYOUT,
 	LANDLOCK_REQUEST_FS_ACCESS,
 	LANDLOCK_REQUEST_NET_ACCESS,
+	LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
+	LANDLOCK_REQUEST_SCOPE_SIGNAL,
 };
 
 /*
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 1decd6f114e8..bdf7adc14e32 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -263,13 +263,27 @@  static int hook_unix_stream_connect(struct sock *const sock,
 	const struct landlock_ruleset *const dom =
 		landlock_get_applicable_domain(landlock_get_current_domain(),
 					       unix_scope);
+	struct lsm_network_audit audit_net = {
+		.sk = other,
+	};
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
+		.audit = {
+			.type = LSM_AUDIT_DATA_NET,
+			.u.net = &audit_net,
+		},
+	};
 
 	/* Quick return for non-landlocked tasks. */
 	if (!dom)
 		return 0;
 
-	if (is_abstract_socket(other) && sock_is_scoped(other, dom))
+	if (is_abstract_socket(other) && sock_is_scoped(other, dom)) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, unix_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -280,6 +294,16 @@  static int hook_unix_may_send(struct socket *const sock,
 	const struct landlock_ruleset *const dom =
 		landlock_get_applicable_domain(landlock_get_current_domain(),
 					       unix_scope);
+	struct lsm_network_audit audit_net = {
+		.sk = other->sk,
+	};
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
+		.audit = {
+			.type = LSM_AUDIT_DATA_NET,
+			.u.net = &audit_net,
+		},
+	};
 
 	if (!dom)
 		return 0;
@@ -291,8 +315,12 @@  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) && sock_is_scoped(other->sk, dom))
+	if (is_abstract_socket(other->sk) && sock_is_scoped(other->sk, dom)) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, unix_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -307,6 +335,13 @@  static int hook_task_kill(struct task_struct *const p,
 {
 	bool is_scoped;
 	const struct landlock_ruleset *dom;
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_SIGNAL,
+		.audit = {
+			.type = LSM_AUDIT_DATA_TASK,
+			.u.tsk = p,
+		},
+	};
 
 	if (cred) {
 		/* Dealing with USB IO. */
@@ -324,8 +359,12 @@  static int hook_task_kill(struct task_struct *const p,
 	is_scoped = domain_is_scoped(dom, landlock_get_task_domain(p),
 				     LANDLOCK_SCOPE_SIGNAL);
 	rcu_read_unlock();
-	if (is_scoped)
+	if (is_scoped) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, signal_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }
@@ -334,6 +373,13 @@  static int hook_file_send_sigiotask(struct task_struct *tsk,
 				    struct fown_struct *fown, int signum)
 {
 	const struct landlock_ruleset *dom;
+	struct landlock_request request = {
+		.type = LANDLOCK_REQUEST_SCOPE_SIGNAL,
+		.audit = {
+			.type = LSM_AUDIT_DATA_TASK,
+			.u.tsk = tsk,
+		},
+	};
 	bool is_scoped = false;
 
 	/* Lock already held by send_sigio() and send_sigurg(). */
@@ -349,8 +395,12 @@  static int hook_file_send_sigiotask(struct task_struct *tsk,
 	is_scoped = domain_is_scoped(dom, landlock_get_task_domain(tsk),
 				     LANDLOCK_SCOPE_SIGNAL);
 	rcu_read_unlock();
-	if (is_scoped)
+	if (is_scoped) {
+		request.layer_plus_one =
+			landlock_match_layer_level(dom, signal_scope) + 1;
+		landlock_log_denial(dom, &request);
 		return -EPERM;
+	}
 
 	return 0;
 }