On Wed, Aug 14, 2024 at 12:22:19AM -0600, Tahera Fahimi wrote:

This patch introduces a new "scoped" attribute to the landlock_ruleset_attr
that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET" to scope
abstract Unix sockets from connecting to a process outside of
the same landlock domain. It implements two hooks, unix_stream_connect
and unix_may_send to enforce this restriction.

Closes: https://github.com/landlock-lsm/linux/issues/7
Signed-off-by: Tahera Fahimi <redacted>

---
v9:
- Editting inline comments.
- Major refactoring in domain_is_scoped() and is_abstract_socket
v8:
- Code refactoring (improve code readability, renaming variable, etc.) based
  on reviews by Mickaël Salaün on version 7.
- Adding warn_on_once to check (impossible) inconsistencies.
- Adding inline comments.
- Adding check_unix_address_format to check if the scoping socket is an abstract
  unix sockets.
v7:
 - Using socket's file credentials for both connected(STREAM) and
   non-connected(DGRAM) sockets.
 - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
   ptrace ensures that if a server's domain is accessible from the client's
   domain (where the client is more privileged than the server), the client
   can connect to the server in all edge cases.
 - Removing debug codes.
v6:
 - Removing curr_ruleset from landlock_hierarchy, and switching back to use
   the same domain scoping as ptrace.
 - code clean up.
v5:
 - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
 - Adding curr_ruleset to hierarachy_ruleset structure to have access from
   landlock_hierarchy to its respective landlock_ruleset.
 - Using curr_ruleset to check if a domain is scoped while walking in the
   hierarchy of domains.
 - Modifying inline comments.
V4:
 - Rebased on Günther's Patch:
   https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/ (local)
   so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
 - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
 - Using socket's file credentials instead of credentials stored in peer_cred
   for datagram sockets. (see discussion in [1])
 - Modifying inline comments.
V3:
 - Improving commit description.
 - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
   purpose, and adding related functions.
 - Changing structure of ruleset based on "scoped".
 - Removing rcu lock and using unix_sk lock instead.
 - Introducing scoping for datagram sockets in unix_may_send.
V2:
 - Removing wrapper functions

[1]https://lore.kernel.org/all/20240610.Aifee5ingugh@digikod.net/ (local)
----

Useless "----"

---
 include/uapi/linux/landlock.h |  27 +++++++
 security/landlock/limits.h    |   3 +
 security/landlock/ruleset.c   |   7 +-
 security/landlock/ruleset.h   |  23 +++++-
 security/landlock/syscalls.c  |  17 +++--
 security/landlock/task.c      | 129 ++++++++++++++++++++++++++++++++++
 6 files changed, 198 insertions(+), 8 deletions(-)

+static bool sock_is_scoped(struct sock *const other,
+			   const struct landlock_ruleset *const dom)

Please rename "dom" to "domain".  Function arguments with full names
make the API more consistent and easier to understand.

+{
+	const struct landlock_ruleset *dom_other;
+
+	/* the credentials will not change */
+	lockdep_assert_held(&unix_sk(other)->lock);
+	dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
+	return domain_is_scoped(dom, dom_other,
+				LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET);
+}
+
+static bool is_abstract_socket(struct sock *const sock)
+{
+	struct unix_address *addr = unix_sk(sock)->addr;
+
+	if (!addr)
+		return false;
+
+	if (addr->len >= offsetof(struct sockaddr_un, sun_path) + 1 &&
+	    addr->name[0].sun_path[0] == '\0')
+		return true;
+
+	return false;

Much better!

+}
+
+static int hook_unix_stream_connect(struct sock *const sock,
+				    struct sock *const other,
+				    struct sock *const newsk)
+{
+	const struct landlock_ruleset *const dom =
+		landlock_get_current_domain();
+
+	/* quick return for non-sandboxed processes */
+	if (!dom)
+		return 0;
+
+	if (is_abstract_socket(other))
+		if (sock_is_scoped(other, dom))

if (is_abstract_socket(other) && sock_is_scoped(other, dom))

(We might want to extend this hook in the future but we'll revise this
notation when needed)

+			return -EPERM;
+
+	return 0;
+}
+
+static int hook_unix_may_send(struct socket *const sock,
+			      struct socket *const other)
+{
+	const struct landlock_ruleset *const dom =
+		landlock_get_current_domain();
+
+	if (!dom)
+		return 0;
+
+	if (is_abstract_socket(other->sk))
+		if (sock_is_scoped(other->sk, dom))

ditto

+			return -EPERM;
+
+	return 0;
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
+	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
+	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
 };
 
 __init void landlock_add_task_hooks(void)
-- 
2.34.1