landlock: Pathname-based UNIX connect() control
Subsystems (7)
v2 cover landlock: Pathname-based UNIX connect() control
Hello!
This patch set introduces a filesystem-based Landlock restriction mechanism for connecting to UNIX domain sockets (or addressing them with sendmsg(2)). It introduces a file system access right for each type of UNIX domain socket:
* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
For the connection-oriented SOCK_STREAM and SOCK_SEQPACKET type sockets, the access right makes the connect(2) operation fail with EACCES, if denied.
SOCK_DGRAM-type UNIX sockets can be used both with connect(2), or by passing an explicit recipient address with every sendmsg(2) invocation. In the latter case, the Landlock check is done when an explicit recipient address is passed to sendmsg(2) and can make sendmsg(2) return EACCES. When UNIX datagram sockets are connected with connect(2), a fixed recipient address is associated with the socket and the check happens during connect(2) and may return EACCES.
## Motivation
Currently, landlocked processes can connect() to named UNIX sockets through the BSD socket API described in unix(7), by invoking socket(2) followed by connect(2) with a suitable struct sockname_un holding the socket's filename. This can come as a surprise for users (e.g. in [1]) and it can be used to escape a sandbox when a Unix service offers command execution (some scenarios were listed by Tingmao Wang in [2]).
The original feature request is at [4].
## Alternatives and Related Work
### Alternative: Use existing LSM hooks
The existing hooks security_unix_stream_connect(), security_unix_may_send() and security_socket_connect() do not give access to the resolved file system path.
Resolving the file system path again within Landlock would in my understanding produce a TOCTOU race, so making the decision based on the struct sockaddr_un contents is not an option.
It is tempting to use the struct path that the listening socket is bound to, which can be acquired through the existing hooks. Unfortunately, the listening socket may have been bound from within a different namespace, and it is therefore a path that can not actually be referenced by the sandboxed program at the time of constructing the Landlock policy. (More details are on the Github issue at [6] and on the LKML at [9]).
### Related work: Scope Control for Pathname Unix Sockets
The motivation for this patch is the same as in Tingmao Wang's patch set for "scoped" control for pathname Unix sockets [2], originally proposed in the Github feature request [5].
In my reply to this patch set [3], I have discussed the differences between these two approaches. On the related discussions on Github
[4] and [5], there was consensus that the scope-based control is complimentary to the file system based control, but does not replace it. Mickael's opening remark on [5] says:
Quoted 3 lines
This scoping would be complementary to #36 which would mainly be about allowing a sandboxed process to connect to a more privileged service (identified with a path).
## Open questions in V2
Seeking feedback on:
- Feedback on the LSM hook name would be appreciated. We realize that not all invocations of the LSM hook are related to connect(2) as the name suggests, but some also happen during sendmsg(2).
- Feedback on the structuring of the Landlock access rights, splitting them up by socket type. (Also naming; they are now consistently called "RESOLVE", but could be named "CONNECT" in the stream and seqpacket cases?)
## Credits
The feature was originally suggested by Jann Horn in [7].
Tingmao Wang and Demi Marie Obenour have taken the initiative to revive this discussion again in [1], [4] and [5] and Tingmao Wang has sent the patch set for the scoped access control for pathname Unix sockets [2].
Justin Suess has sent the patch for the LSM hook in [8].
Ryan Sullivan has started on an initial implementation and has brought up relevant discussion points on the Github issue at [4] that lead to the current approach.
[1] https://lore.kernel.org/landlock/[email protected]/
[2] Tingmao Wang's "Implemnet scope control for pathname Unix sockets"
https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] Github issue for FS-based control for named Unix sockets:
https://github.com/landlock-lsm/linux/issues/36
[5] Github issue for scope-based restriction of named Unix sockets:
https://github.com/landlock-lsm/linux/issues/51
[6] https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
[7] https://lore.kernel.org/linux-security-module/CAG48ez3NvVnonOqKH4oRwRqbSOLO0p9djBqgvxVwn6gtGQBPcw@mail.gmail.com/
[8] Patch for the LSM hook:
https://lore.kernel.org/all/[email protected]/
[9] https://lore.kernel.org/all/[email protected]/
---
## Older versions of this patch set
V1: https://lore.kernel.org/all/[email protected]/
Changes in V2:
* Send Justin Suess's LSM hook patch together with the Landlock implementation
* LSM hook: Pass type and flags parameters to the hook, to make the access right more generally usable across LSMs, per suggestion from Paul Moore (Implemented by Justin)
* Split the access right into the three types of UNIX domain sockets:
SOCK_STREAM, SOCK_DGRAM and SOCK_SEQPACKET.
* selftests: More exhaustive tests.
* Removed a minor commit from V1 which adds a missing close(fd) to a test (it is already in the mic-next branch)
Günther Noack (4):
landlock: Control pathname UNIX domain socket resolution by path samples/landlock: Add support for named UNIX domain socket
restrictions
landlock/selftests: Test named UNIX domain socket restrictions
landlock: Document FS access rights for pathname UNIX sockets
Justin Suess (1):
lsm: Add hook unix_path_connect
Documentation/userspace-api/landlock.rst | 25 ++- include/linux/lsm_hook_defs.h | 4 + include/linux/security.h | 11 + include/uapi/linux/landlock.h | 10 + net/unix/af_unix.c | 9 + samples/landlock/sandboxer.c | 18 +- security/landlock/access.h | 2 +- security/landlock/audit.c | 6 + security/landlock/fs.c | 34 ++- security/landlock/limits.h | 2 +- security/landlock/syscalls.c | 2 +- security/security.c | 20 ++ tools/testing/selftests/landlock/base_test.c | 2 +- tools/testing/selftests/landlock/fs_test.c | 225 +++++++++++++++++-- 14 files changed, 344 insertions(+), 26 deletions(-)
Signature · 1 line
2.52.0
PATCH v2 1/5 lsm: Add hook unix_path_connect
From: Justin Suess <[email protected]>
Adds an LSM hook unix_path_connect.
This hook is called to check the path of a named unix socket before a connection is initiated.
--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)
include/linux/lsm_hook_defs.h +4 -0
@@ -317,6 +317,10 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred, LSM_HOOK(int, 0, watch_key, struct key *key)#endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +LSM_HOOK(int, 0, unix_path_connect, const struct path *path, int type, int flags) +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_NETWORK LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other, struct sock *newsk)
include/linux/security.h +11 -0
@@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) }#endif /* CONFIG_SECURITY_NETWORK */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) + +int security_unix_path_connect (const struct path *path,int type,int flags); + +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +static inline int security_unix_path_connect (const struct path *path,int type,int flags) +{ +return 0 ; +} +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_INFINIBAND int security_ib_pkey_access (void *sec,u64 subnet_prefix,u16 pkey);int security_ib_endport_manage_subnet (void *sec,const char *name,u8 port_num);
net/unix/af_unix.c +9 -0
@@ -1226,6 +1226,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,if (!S_ISSOCK (inode->i_mode)) goto path_put; +/* +* We call the hook because we know that the inode is a socket +* and we hold a valid reference to it via the path. +*/ + err =security_unix_path_connect (&path, type, flags); +if (err) + goto path_put; + + err = -ECONNREFUSED ; sk =unix_find_socket_byinode (inode);if (!sk) goto path_put;
security/security.c +20 -0
@@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)#endif /* CONFIG_SECURITY_NETWORK */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +/* +* security_unix_path_connect() - Check if a named AF_UNIX socket can connect +* @path: path of the socket being connected to +* @type: type of the socket +* @flags: flags associated with the socket +* +* This hook is called to check permissions before connecting to a named +* AF_UNIX socket. +* +* Return: Returns 0 if permission is granted. +*/ +int security_unix_path_connect (const struct path *path,int type,int flags) +{ +return call_int_hook (unix_path_connect, path, type, flags); +} +EXPORT_SYMBOL (security_unix_path_connect); + +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_INFINIBAND /** * security_ib_pkey_access() - Check if access to an IB pkey is allowed
Signature · 1 line
2.52.0
Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 94 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)include/linux/lsm_hook_defs.h +4
-0@@ -317,6 +317,10 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred, LSM_HOOK(int, 0, watch_key, struct key *key)#endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +LSM_HOOK(int, 0, unix_path_connect, const struct path *path, int type, int flags) +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_NETWORK LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other, struct sock *newsk)include/linux/security.h +11
-0@@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) }#endif /* CONFIG_SECURITY_NETWORK */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) + +int security_unix_path_connect (const struct path *path,int type,int flags); + +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +static inline int security_unix_path_connect (const struct path *path,int type,int flags) +{ +return 0 ; +} +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_INFINIBAND int security_ib_pkey_access (void *sec,u64 subnet_prefix,u16 pkey);int security_ib_endport_manage_subnet (void *sec,const char *name,u8 port_num);net/unix/af_unix.c +9
-0@@ -1226,6 +1226,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,if (!S_ISSOCK (inode->i_mode)) goto path_put; +/* +* We call the hook because we know that the inode is a socket +* and we hold a valid reference to it via the path. +*/ + err =security_unix_path_connect (&path, type, flags); +if (err) + goto path_put; + + err = -ECONNREFUSED ; sk =unix_find_socket_byinode (inode);if (!sk) goto path_put;security/security.c +13
-0@@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)#endif /* CONFIG_SECURITY_NETWORK */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +/* +* security_unix_path_connect() - Check if a named AF_UNIX socket can connect +* @path: path of the socket being connected to +* @type: type of the socket +* @flags: flags associated with the socket +* +* This hook is called to check permissions before connecting to a named +* AF_UNIX socket. +* +* Return: Returns 0 if permission is granted. +*/ +int security_unix_path_connect (const struct path *path,int type,int flags)
security/security.c int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) · line 4756
Quoted 10 lines
security/security.c +7
-0+{ +return call_int_hook (unix_path_connect, path, type, flags); +} +EXPORT_SYMBOL (security_unix_path_connect); + +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_INFINIBAND /** * security_ib_pkey_access() - Check if access to an IB pkey is allowed
Just for awareness,
I'm considering renaming this hook to unix_socket_path_lookup, since as Günther pointed out this hook is not just hit on connect, but also on sendmsg.
Justin
Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 4 lines · Justin Suess
Just for awareness,
I'm considering renaming this hook to
unix_socket_path_lookup, since as Günther pointed out this hook is not just hit on connect, but also on sendmsg.
+1 I would be in favor of this.
(In doubt, Paul Moore has the last word on LSM hook naming. I believe that both "lookup" and "resolve" are being used exchangeably to refer to path lookups. (e.g. see the path_resolution(7) man page [1]))
–Günther
[1] https://man7.org/linux/man-pages/man7/path_resolution.7.html
Paul Moore
maintainerRe: [PATCH v2 1/5] lsm: Add hook unix_path_connect
In reply to Justin SuessQuoted 16 lines · Justin Suess
Quoted 15 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)
...
Quoted 17 lines
Quoted 17 lines
security/security.c +17
-0+#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +/* +* security_unix_path_connect() - Check if a named AF_UNIX socket can connect +* @path: path of the socket being connected to +* @type: type of the socket +* @flags: flags associated with the socket +* +* This hook is called to check permissions before connecting to a named +* AF_UNIX socket. +* +* Return: Returns 0 if permission is granted. +*/ +int security_unix_path_connect (const struct path *path,int type,int flags) +{ +return call_int_hook (unix_path_connect, path, type, flags); +} +EXPORT_SYMBOL (security_unix_path_connect);
...
Quoted 2 lines
I'm considering renaming this hook to
unix_socket_path_lookup, since as Günther pointed out this hook is not just hit on connect, but also on sendmsg.
I'm not bothered too much by this, either _path_connect() or _path_lookup() is okay; please don't use security_unix_socket_path_lookup(), that's longer than it needs to be, if you've got "_unix_" in there we know you're talking about a socket :)
While I don't want us to do it often, we can always change established hook names if the names end up being really awful or misleading. We've done it in the past.
It would be nice if somehow the hook name reflected the fact that it is called on the "client" side of the connection, and not the "server" side, but I wouldn't use either of those terms (client or server), and to be honest I can't think of anything better than _path_lookup() at the moment.
Signature · 1 line
paul-moore.com
Paul Moore
maintainerRe: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 40 lines · Paul Moore
Quoted 16 lines · Justin Suess
Quoted 15 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)...
Quoted 17 lines
Quoted 17 lines
security/security.c +17
-0+#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +/* +* security_unix_path_connect() - Check if a named AF_UNIX socket can connect +* @path: path of the socket being connected to +* @type: type of the socket +* @flags: flags associated with the socket +* +* This hook is called to check permissions before connecting to a named +* AF_UNIX socket. +* +* Return: Returns 0 if permission is granted. +*/ +int security_unix_path_connect (const struct path *path,int type,int flags) +{ +return call_int_hook (unix_path_connect, path, type, flags); +} +EXPORT_SYMBOL (security_unix_path_connect);...
Quoted 4 lines
Quoted 2 lines
I'm considering renaming this hook to
unix_socket_path_lookup, since as Günther pointed out this hook is not just hit on connect, but also on sendmsg.I'm not bothered too much ...
I forgot to add that I know you're likely going to do another revision to this patchset to rename the hook, but I would suggest waiting until the AppArmor folks have had a chance to look at the hook. I want to make sure the new hook is reasonable and suitably generic for a path-based LSM, and while I suspect it is, having another set of path-based LSM eyes review the hook would be a very good thing.
Signature · 1 line
paul-moore.com
Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
In reply to Günther NoackQuoted 58 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)include/linux/lsm_hook_defs.h +4
-0@@ -317,6 +317,10 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred, LSM_HOOK(int, 0, watch_key, struct key *key)#endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) +LSM_HOOK(int, 0, unix_path_connect, const struct path *path, int type, int flags) +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_NETWORK LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other, struct sock *newsk)include/linux/security.h +11
-0@@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) }#endif /* CONFIG_SECURITY_NETWORK */ +#if defined(CONFIG_SECURITY_NETWORK ) && defined(CONFIG_SECURITY_PATH ) + +int security_unix_path_connect (const struct path *path,int type,int flags); + +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +static inline int security_unix_path_connect (const struct path *path,int type,int flags) +{ +return 0 ; +} +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */ +#ifdef CONFIG_SECURITY_INFINIBAND int security_ib_pkey_access (void *sec,u64 subnet_prefix,u16 pkey);int security_ib_endport_manage_subnet (void *sec,const char *name,u8 port_num);net/unix/af_unix.c +0
-0
net/unix/af_unix.c static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len, · line 1235
Quoted 10 lines
net/unix/af_unix.c +7
-0@@ -1226,6 +1226,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,if (!S_ISSOCK (inode->i_mode)) goto path_put; +/* +* We call the hook because we know that the inode is a socket +* and we hold a valid reference to it via the path. +*/ + err =security_unix_path_connect (&path, type, flags); +if (err) + goto path_put;
Couldn't we try reflowing the code here so the path is passed to security_unix_stream_connect() and security_unix_may_send() so that all LSMs get the same data and we don't have to have different LSMs hooks into different callpaths that effectively do the same thing.
I mean the objects are even in two completely different states between those hooks. Even what type of sockets get a call to the LSM is different between those two hooks.
Paul Moore
maintainerRe: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 16 lines · Christian Brauner
Quoted 15 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)
...
Quoted 8 lines
Quoted 8 lines
net/unix/af_unix.c +0
-0@@ -1226,6 +1226,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,if (!S_ISSOCK (inode->i_mode)) goto path_put;
Quoted 9 lines
net/unix/af_unix.c static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len, · line 1235
Quoted 7 lines
net/unix/af_unix.c +7
-0+/* +* We call the hook because we know that the inode is a socket +* and we hold a valid reference to it via the path. +*/ + err =security_unix_path_connect (&path, type, flags); +if (err) + goto path_put;Couldn't we try reflowing the code here so the path is passed ...
It would be good if you could be a bit more specific about your desires here. Are you talking about changing the unix_find_other()/unix_find_bsd() code path such that the path is available to unix_find_other() callers and not limited to the unix_find_bsd() scope?
Quoted 8 lines
... to
security_unix_stream_connect()andsecurity_unix_may_send()so that all LSMs get the same data and we don't have to have different LSMs hooks into different callpaths that effectively do the same thing.I mean the objects are even in two completely different states between those hooks. Even what type of sockets get a call to the LSM is different between those two hooks.
I'm working on the assumption that you are talking about changing the UNIX socket code so that the path info is available to the existing _may_send() and _stream_connect() hooks. If that isn't the case, and you're thinking of something different, disregard my comments below.
In both the unix_dgram_{connect(),sendmsg()}, aka security_unix_may_send(), cases and the unix_stream_connect(), aka security_unix_stream_connect(), case the call to unix_find_other() is done to lookup the other end of the communication channel, which does seem reasonably consistent to me. Yes, of course, once you start getting into the specifics of the UNIX socket handling the unix_dgram_ and unix_stream_ cases are very different, including their corresponding existing LSM hooks, but that doesn't mean in the context of unix_find_bsd() that security_unix_path_connect() doesn't have value.
The alternative would be some rather serious surgery in af_unix.c to persist the path struct from unix_find_bsd() until the later LSM hooks are executed. It's certainly not impossible, but I'm not sure it is necessary or desirable at this point in time. LSMs that wish to connect the information from _unix_path_connect() to either _unix_stream_connect() or _unix_may_send() can do so today without needing to substantially change af_unix.c.
Signature · 1 line
paul-moore.com
Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 69 lines · Paul Moore
Quoted 16 lines · Christian Brauner
Quoted 15 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)...
Quoted 8 lines
Quoted 8 lines
net/unix/af_unix.c +0
-0@@ -1226,6 +1226,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,if (!S_ISSOCK (inode->i_mode)) goto path_put;Quoted 9 lines
net/unix/af_unix.c static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len, · line 1235
Quoted 7 lines
net/unix/af_unix.c +7
-0+/* +* We call the hook because we know that the inode is a socket +* and we hold a valid reference to it via the path. +*/ + err =security_unix_path_connect (&path, type, flags); +if (err) + goto path_put;Couldn't we try reflowing the code here so the path is passed ...
It would be good if you could be a bit more specific about your desires here. Are you talking about changing the unix_find_other()/unix_find_bsd() code path such that the path is available to
unix_find_other()callers and not limited to theunix_find_bsd()scope?Quoted 8 lines
... to
security_unix_stream_connect()andsecurity_unix_may_send()so that all LSMs get the same data and we don't have to have different LSMs hooks into different callpaths that effectively do the same thing.I mean the objects are even in two completely different states between those hooks. Even what type of sockets get a call to the LSM is different between those two hooks.
I'm working on the assumption that you are talking about changing the UNIX socket code so that the path info is available to the existing
_may_send()and_stream_connect()hooks. If that isn't the case, and you're thinking of something different, disregard my comments below.In both the unix_dgram_{connect(),sendmsg()}, aka
security_unix_may_send(), cases and theunix_stream_connect(), akasecurity_unix_stream_connect(), case the call tounix_find_other()is done to lookup the other end of the communication channel, which does seem reasonably consistent to me. Yes, of course, once you start getting into the specifics of the UNIX socket handling theunix_dgram_andunix_stream_cases are very different, including their corresponding existing LSM hooks, but that doesn't mean in the context ofunix_find_bsd()thatsecurity_unix_path_connect()doesn't have value.
Quoted 7 lines
The alternative would be some rather serious surgery in
af_unix.cto persist the path struct fromunix_find_bsd()until the later LSM hooks are executed. It's certainly not impossible, but I'm not sure it is necessary or desirable at this point in time. LSMs that wish to connect the information from_unix_path_connect()to either_unix_stream_connect()or_unix_may_send()can do so today without needing to substantially changeaf_unix.c.
Thanks for the review, Christan and Paul!
I am also unconvinced by the approach. It has also crossed my mind before though, and my reasoning is as follows:
For reference, the function call hierarchy in af_unix.c is:
* unix_dgram_connect() (*)
* unix_find_other()
* unix_find_bsd()
* security_unix_may_send()
* unix_dgram_sendmsg() (*)
* unix_find_other()
* unix_find_bsd()
* security_unix_may_send()
* unix_stream_connect() (*)
* unix_find_other()
* unix_find_bsd()
* security_unix_stream_connect()
In my understanding, the hypothetical implementation would be:
* allocate a struct path on the stack of these proto_ops hook functions (marked with (*) above)
* pass a pointer to that path down to unix_find_other(), only to be filled out in the case that this is a pathname UNIX socket (not an abstract UNIX socket)
* pass a const pointer to that path to the LSM hooks
and then the LSM hooks would have to check whether the struct path has non-zero pointers and could do the check.
This has the upside that it does not introduce a new LSM hook, but only adds a "path" parameter to two existing LSM hooks.
On the other side, I see the following drawbacks:
* The more serious surgery in af_unix, which Paul also discussed:
The function interface to unix_find_other() would need additional parameters for the sole purpose of supporting these LSM hooks and the refcounting of the struct path would have to be done in three functions instead of just in one. That would complicate the af_unix.c logic, even when CONFIG_SECURITY_PATH is not set.
* The cases in which we pass a non-zero path argument to the LSM hooks would have surprising semantics IMHO, because it is not always set:
* If a UNIX dgram user uses connect(2) and then calls sendmsg(2) without explicit recipient address, unix_dgram_sendmsg() would *not* do the look up any more and we can not provide the path to the security_unix_may_send() hook.
* For abstract UNIX sockets it is not set either, of course.
The path argument to the LSM hook would be present in the exact same cases where we now call the new UNIX path lookup LSM hook, but it would be invoked with a delay.
* Some properties of the resolved socket are still observable to userspace:
When we only pass the path to a later LSM hook, there are a variety of additional error case checks in af_unix.c which are based on the "other" socket which we looked up through the path. Examples:
* was other shutdown(2)? (ECONNREFUSED on connect or EPIPE on dgram_sendmsg)
* does other support SO_PASSRIGHTS (fd passing)? (EPERM on dgram_sendmsg)
* would sendmsg pass sk_filter() (on dgram_sendmsg)
For a LSM policy that is supposed to restrict the resolution of a UNIX socket by path, I would not expect such properties of the resolved socket to be observable?
(And we also can't fix this up in the LSM by returning a matching error code, because at least unix_dgram_sendmsg() returns multiple different error codes in these error cases.)
I would prefer if the correctness of our LSM did not depend on keeping track of the error scenarios in af_unix.c. This seems brittle.
Overall, I am not convinced that using pre-existing hooks is the right way and I would prefer the approach where we have a more dedicated LSM hook for the path lookup.
Does that seem reasonable? Let me know what you think.
–Günther
Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 20 lines · Günther Noack
Quoted 19 lines · Paul Moore
Quoted 16 lines · Christian Brauner
Quoted 15 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)...
...
Quoted 15 lines
* Some properties of the resolved socket are still observable to userspace:
When we only pass the path to a later LSM hook, there are a variety of additional error case checks in
af_unix.cwhich are based on the "other" socket which we looked up through the path. Examples:* was other shutdown(2)? (ECONNREFUSED on connect or EPIPE on
dgram_sendmsg)
* does other supportSO_PASSRIGHTS(fd passing)? (EPERM ondgram_sendmsg)
* would sendmsg passsk_filter()(ondgram_sendmsg)For a LSM policy that is supposed to restrict the resolution of a UNIX socket by path, I would not expect such properties of the resolved socket to be observable?
Quoted 7 lines
(And we also can't fix this up in the LSM by returning a matching error code, because at least
unix_dgram_sendmsg()returns multiple different error codes in these error cases.)I would prefer if the correctness of our LSM did not depend on keeping track of the error scenarios in
af_unix.c. This seems brittle.
Indeed so.
Quoted 7 lines
Overall, I am not convinced that using pre-existing hooks is the right way and I would prefer the approach where we have a more dedicated LSM hook for the path lookup.
Does that seem reasonable? Let me know what you think.
–Günther
Having a dedicated LSM hook for all path lookups is definitely my preferred approach. Could this allow limiting directory traversal as well?
Signature · 2 lines
Sincerely, Demi Marie Obenour (she/her/hers)
Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
Quoted 6 lines · Demi Marie Obenour
Quoted 3 lines · Günther Noack
I would prefer if the correctness of our LSM did not depend on keeping track of the error scenarios in
af_unix.c. This seems brittle.Indeed so.
Thanks for the support!
Quoted 11 lines
Quoted 7 lines
Overall, I am not convinced that using pre-existing hooks is the right way and I would prefer the approach where we have a more dedicated LSM hook for the path lookup.
Does that seem reasonable? Let me know what you think.
–Günther
Having a dedicated LSM hook for all path lookups is definitely my preferred approach. Could this allow limiting directory traversal as well?
No, this does not limit all path lookups, in the sense of what was discussed in the thread at [1]. (I assume this is what you meant?)
The LSM hook proposed here is only about the lookup of named UNIX domain sockets, as it happens when clients pass a struct sockaddr_un to connect(2) or sendmsg(2).
–Günther
Paul Moore
maintainerRe: [PATCH v2 1/5] lsm: Add hook unix_path_connect
In reply to Günther NoackQuoted 18 lines · Günther Noack
Quoted 17 lines · Paul Moore
Quoted 16 lines · Christian Brauner
Quoted 15 lines · Günther Noack
From: Justin Suess <[email protected]>
Adds an LSM hook
unix_path_connect.This hook is called to check the path of a named unix socket before a connection is initiated.
Cc Günther Noack <[email protected]>Signed-off-by Justin Suess <[email protected]>--- include/linux/lsm_hook_defs.h | 4 ++++ include/linux/security.h | 11 +++++++++++ net/unix/af_unix.c | 9 +++++++++ security/security.c | 20 ++++++++++++++++++++ 4 files changed, 44 insertions(+)
...
Quoted 3 lines
On the other side, I see the following drawbacks:
* The more serious surgery in
af_unix, which Paul also discussed:
Not to take away from what Günther already mentioned, but my concern about extending the path beyond the unix_find_bsd() function for the sake of the LSM is that history has shown that the easiest (this is very much a relative statement) approach towards acceptance of a new LSM hook is to keep the addition/patch as small as possible while still being useful. Making the addition of a new LSM hook dependent on significant changes outside of the security/ directory often results in failure.
Quoted 5 lines
Overall, I am not convinced that using pre-existing hooks is the right way and I would prefer the approach where we have a more dedicated LSM hook for the path lookup.
Does that seem reasonable? Let me know what you think.
I believe it's definitely the "path" (sorry) of least resistance.
Signature · 1 line
paul-moore.com
PATCH v2 2/5 landlock: Control pathname UNIX domain socket resolution by path
In reply to Günther Noack* Add new access rights which control the look up operations for named UNIX domain sockets. The resolution happens during connect() and sendmsg() (depending on socket type).
* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
* Hook into the path lookup in unix_find_bsd() in af_unix.c, using a LSM hook. Make policy decisions based on the new access rights
* Increment the Landlock ABI version.
* Minor test adaptions to keep the tests working.
--- include/uapi/linux/landlock.h | 10 ++++++ security/landlock/access.h | 2 +- security/landlock/audit.c | 6 ++++ security/landlock/fs.c | 34 +++++++++++++++++++- security/landlock/limits.h | 2 +- security/landlock/syscalls.c | 2 +- tools/testing/selftests/landlock/base_test.c | 2 +- tools/testing/selftests/landlock/fs_test.c | 7 ++-- 8 files changed, 58 insertions(+), 7 deletions(-)
include/uapi/linux/landlock.h +10 -0
@@ -216,6 +216,13 @@ struct landlock_net_port_attr {* :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with * ``O_TRUNC``. This access right is available since the third version of the * Landlock ABI. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM: Connect to named +* :manpage:`unix(7)` ``SOCK_STREAM`` sockets. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM: Send messages to named +* :manpage:`unix(7)` ``SOCK_DGRAM`` sockets or connect to them using +* :manpage:`connect(2)`. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET: Connect to named +* :manpage:`unix(7)` ``SOCK_SEQPACKET`` sockets. * * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
@@ -321,6 +328,9 @@ struct landlock_net_port_attr {#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM (1ULL << 16) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM (1ULL << 17) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET (1ULL << 18)/* clang-format on */ /**
security/landlock/access.h +1 -1
@@ -34,7 +34,7 @@ LANDLOCK_ACCESS_FS_IOCTL_DEV)/* clang-format on */ -typedef u16 access_mask_t ; +typedef u32 access_mask_t ;/* Makes sure all filesystem access rights can be stored. */ static_assert (BITS_PER_TYPE (access_mask_t) >=LANDLOCK_NUM_ACCESS_FS );
security/landlock/audit.c +6 -0
@@ -37,6 +37,12 @@ static const char *const fs_access_strings[] = { [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer", [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate", [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM )] = + "fs.resolve_unix_stream", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM )] = + "fs.resolve_unix_dgram", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET )] = +"fs.resolve_unix_seqpacket" , };static_assert (ARRAY_SIZE (fs_access_strings) ==LANDLOCK_NUM_ACCESS_FS );
security/landlock/fs.c +33 -1
@@ -9,6 +9,7 @@* Copyright © 2023-2024 Google LLC */ +#include "linux/net.h" #include <asm/ioctls.h> #include <kunit/test.h> #include <linux/atomic.h>
@@ -314,7 +315,10 @@ static struct landlock_object *get_inode_object(struct inode *const inode)LANDLOCK_ACCESS_FS_WRITE_FILE | \LANDLOCK_ACCESS_FS_READ_FILE | \LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) +LANDLOCK_ACCESS_FS_IOCTL_DEV | \ +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)/* clang-format on */ /*
@@ -1588,6 +1592,33 @@ static int hook_path_truncate(const struct path *const path)return current_check_access_path (path,LANDLOCK_ACCESS_FS_TRUNCATE ); } +static int hook_unix_path_connect (const struct path *const path,int type, +int flags) +{ +access_mask_t access_request =0 ; + +/* Lookup for the purpose of saving coredumps is OK. */ +if (flags &SOCK_COREDUMP ) +return 0 ; + +switch (type) { +case SOCK_STREAM : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM ; +break ; +case SOCK_DGRAM : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM ; +break ; +case SOCK_SEQPACKET : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET ; +break ; + } + +if (!access_request) +return 0 ; + +return current_check_access_path (path, access_request); +} +/* File hooks */ /**
@@ -1872,6 +1903,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {LSM_HOOK_INIT (path_unlink, hook_path_unlink),LSM_HOOK_INIT (path_rmdir, hook_path_rmdir),LSM_HOOK_INIT (path_truncate, hook_path_truncate), +LSM_HOOK_INIT (unix_path_connect, hook_unix_path_connect),LSM_HOOK_INIT (file_alloc_security, hook_file_alloc_security), LSM_HOOK_INIT(file_open, hook_file_open),
security/landlock/limits.h +1 -1
@@ -19,7 +19,7 @@#define LANDLOCK_MAX_NUM_LAYERS 16#define LANDLOCK_MAX_NUM_RULES U32_MAX -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
security/landlock/syscalls.c +1 -1
@@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {* Documentation/userspace-api/landlock.rst should be updated to reflect the * UAPI change. */ -const int landlock_abi_version =7 ; +const int landlock_abi_version =8 ;/** * sys_landlock_create_ruleset - Create a new ruleset
tools/testing/selftests/landlock/base_test.c +1 -1
@@ -76,7 +76,7 @@ TEST(abi_version)const struct landlock_ruleset_attr ruleset_attr = { .handled_access_fs =LANDLOCK_ACCESS_FS_READ_FILE , }; -ASSERT_EQ (7 ,landlock_create_ruleset (NULL ,0 , +ASSERT_EQ (8 ,landlock_create_ruleset (NULL ,0 ,LANDLOCK_CREATE_RULESET_VERSION )); ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
tools/testing/selftests/landlock/fs_test.c +5 -2
@@ -575,9 +575,12 @@ TEST_F_FORK(layout1, inval) LANDLOCK_ACCESS_FS_WRITE_FILE | \ LANDLOCK_ACCESS_FS_READ_FILE | \ LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) + LANDLOCK_ACCESS_FS_IOCTL_DEV | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET) -#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV +#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET#define ACCESS_ALL ( \ ACCESS_FILE | \
Signature · 1 line
2.52.0
Re: [PATCH v2 2/5] landlock: Control pathname UNIX domain socket resolution by path
Quoted 6 lines · Günther Noack
* Add new access rights which control the look up operations for named UNIX domain sockets. The resolution happens during
connect()andsendmsg()(depending on socket type).
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
Might be a crazy thought but would it be better to implement the STREAM/DGRAM/SEQPACKET as an add_rule flag rather than as a separate access right? There are other types of address families like AF_CAN, AF_BLUETOOTH, AF_VSOCK that support multiple socket types.
This saves us on access right numbers if they get added in the future to landlock.
So we could have:
LANDLOCK_ADD_RULE_SOCK_STREAMLANDLOCK_ADD_RULE_SOCK_DGRAMLANDLOCK_ADD_RULE_SOCK_SEQPACKET
and use it as such:
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, &path_beneath_for_unix_socket, LANDLOCK_ADD_RULE_SOCK_STREAM | LANDLOCK_ADD_RULE_SOCK_DGRAM);
For address families with only one socket type (ie tcp and udp), the socket family be implied, which keeps backward compatibility w/ the existing tcp access right.
This way, we don't have to make completely separate access rights for future socket families. So we could add a single access right for bluetooth, for instance, and distinguish which socket families we give it with the LANDLOCK_ADD_RULE_SOCK_* flags.
We'd have to track the SOCK_(socket_type) for unix sockets as we gather access rights. But afaik unix sockets should be the only socket type that has to deal with tree traversal.
Quoted 203 lines
* Hook into the path lookup in
unix_find_bsd()inaf_unix.c, using a LSM hook. Make policy decisions based on the new access rights
* Increment the Landlock ABI version.
* Minor test adaptions to keep the tests working.Cc Justin Suess <[email protected]>Cc Mickaël Salaün <[email protected]>Suggested-by Jann Horn <[email protected]>Signed-off-by Günther Noack <[email protected]>--- include/uapi/linux/landlock.h | 10 ++++++ security/landlock/access.h | 2 +- security/landlock/audit.c | 6 ++++ security/landlock/fs.c | 34 +++++++++++++++++++- security/landlock/limits.h | 2 +- security/landlock/syscalls.c | 2 +- tools/testing/selftests/landlock/base_test.c | 2 +- tools/testing/selftests/landlock/fs_test.c | 7 ++-- 8 files changed, 58 insertions(+), 7 deletions(-)include/uapi/linux/landlock.h +10
-0@@ -216,6 +216,13 @@ struct landlock_net_port_attr {* :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with * ``O_TRUNC``. This access right is available since the third version of the * Landlock ABI. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM: Connect to named +* :manpage:`unix(7)` ``SOCK_STREAM`` sockets. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM: Send messages to named +* :manpage:`unix(7)` ``SOCK_DGRAM`` sockets or connect to them using +* :manpage:`connect(2)`. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET: Connect to named +* :manpage:`unix(7)` ``SOCK_SEQPACKET`` sockets. * * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as @@ -321,6 +328,9 @@ struct landlock_net_port_attr {#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM (1ULL << 16) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM (1ULL << 17) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET (1ULL << 18)/* clang-format on */ /** security/landlock/access.h +1
-1@@ -34,7 +34,7 @@ LANDLOCK_ACCESS_FS_IOCTL_DEV)/* clang-format on */ -typedef u16 access_mask_t ; +typedef u32 access_mask_t ;/* Makes sure all filesystem access rights can be stored. */ static_assert (BITS_PER_TYPE (access_mask_t) >=LANDLOCK_NUM_ACCESS_FS );security/landlock/audit.c +6
-0@@ -37,6 +37,12 @@ static const char *const fs_access_strings[] = { [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer", [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate", [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM )] = + "fs.resolve_unix_stream", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM )] = + "fs.resolve_unix_dgram", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET )] = +"fs.resolve_unix_seqpacket" , };static_assert (ARRAY_SIZE (fs_access_strings) ==LANDLOCK_NUM_ACCESS_FS );security/landlock/fs.c +33
-1@@ -9,6 +9,7 @@* Copyright © 2023-2024 Google LLC */ +#include "linux/net.h" #include <asm/ioctls.h> #include <kunit/test.h> #include <linux/atomic.h> @@ -314,7 +315,10 @@ static struct landlock_object *get_inode_object(struct inode *const inode)LANDLOCK_ACCESS_FS_WRITE_FILE | \LANDLOCK_ACCESS_FS_READ_FILE | \LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) +LANDLOCK_ACCESS_FS_IOCTL_DEV | \ +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)/* clang-format on */ /* @@ -1588,6 +1592,33 @@ static int hook_path_truncate(const struct path *const path)return current_check_access_path (path,LANDLOCK_ACCESS_FS_TRUNCATE ); } +static int hook_unix_path_connect (const struct path *const path,int type, +int flags) +{ +access_mask_t access_request =0 ; + +/* Lookup for the purpose of saving coredumps is OK. */ +if (flags &SOCK_COREDUMP ) +return 0 ; + +switch (type) { +case SOCK_STREAM : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM ; +break ; +case SOCK_DGRAM : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM ; +break ; +case SOCK_SEQPACKET : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET ; +break ; + } + +if (!access_request) +return 0 ; + +return current_check_access_path (path, access_request); +} +/* File hooks */ /** @@ -1872,6 +1903,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {LSM_HOOK_INIT (path_unlink, hook_path_unlink),LSM_HOOK_INIT (path_rmdir, hook_path_rmdir),LSM_HOOK_INIT (path_truncate, hook_path_truncate), +LSM_HOOK_INIT (unix_path_connect, hook_unix_path_connect),LSM_HOOK_INIT (file_alloc_security, hook_file_alloc_security), LSM_HOOK_INIT(file_open, hook_file_open),security/landlock/limits.h +1
-1@@ -19,7 +19,7 @@#define LANDLOCK_MAX_NUM_LAYERS 16#define LANDLOCK_MAX_NUM_RULES U32_MAX -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)security/landlock/syscalls.c +1
-1@@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {* Documentation/userspace-api/landlock.rst should be updated to reflect the * UAPI change. */ -const int landlock_abi_version =7 ; +const int landlock_abi_version =8 ;/** * sys_landlock_create_ruleset - Create a new ruleset tools/testing/selftests/landlock/base_test.c +1
-1@@ -76,7 +76,7 @@ TEST(abi_version)const struct landlock_ruleset_attr ruleset_attr = { .handled_access_fs =LANDLOCK_ACCESS_FS_READ_FILE , }; -ASSERT_EQ (7 ,landlock_create_ruleset (NULL ,0 , +ASSERT_EQ (8 ,landlock_create_ruleset (NULL ,0 ,LANDLOCK_CREATE_RULESET_VERSION )); ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,tools/testing/selftests/landlock/fs_test.c +5
-2@@ -575,9 +575,12 @@ TEST_F_FORK(layout1, inval) LANDLOCK_ACCESS_FS_WRITE_FILE | \ LANDLOCK_ACCESS_FS_READ_FILE | \ LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) + LANDLOCK_ACCESS_FS_IOCTL_DEV | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET) -#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV +#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET#define ACCESS_ALL ( \ ACCESS_FILE | \
Mickaël Salaün
maintainerRe: [PATCH v2 2/5] landlock: Control pathname UNIX domain socket resolution by path
Quoted 11 lines · Justin Suess
Quoted 6 lines · Günther Noack
* Add new access rights which control the look up operations for named UNIX domain sockets. The resolution happens during
connect()andsendmsg()(depending on socket type).
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKETMight be a crazy thought but would it be better to implement the STREAM/DGRAM/SEQPACKET as an
add_ruleflag rather than as a separate access right? There are other types of address families likeAF_CAN,AF_BLUETOOTH,AF_VSOCKthat support multiple socket types.
There should only be one LANDLOCK_ACCESS_FS_RESOLVE_UNIX, not one per stream/dgram/seqpacket.
I guess there is no other type of address families that can be tied to the filesystem, so there is no need for a more generic approach. Moreover, each new access right should come with a good test coverage, so adding a generic access right would be too much work.
The add_rule flags should make sense for any kind of rule type e.g., related to how the rule is applied. We also have a lot of flexibility with the landlock_*_attr structs where we can add new fields.
FYI, this patch series will make it possible to restrict socket creation according to their properties:
https://lore.kernel.org/all/[email protected]/
Quoted 3 lines
This saves us on access right numbers if they get added in the future to landlock.
Even if we reach 64 access rights per type, we can still add new types. Anyway, I guess this would not happen because more specific landlock_*_attr would probably fit best.
Quoted 6 lines
So we could have:
LANDLOCK_ADD_RULE_SOCK_STREAMLANDLOCK_ADD_RULE_SOCK_DGRAMLANDLOCK_ADD_RULE_SOCK_SEQPACKET
This would be the wrong semantic because users don't really care about the stream/dgram/seqpacket type but about the path name and the peer.
If we want to restrict other socket types, we would probably use landlock_net_port_attr or create a new struct if necessary.
Quoted 224 lines
and use it as such:
landlock_add_rule(ruleset_fd,
LANDLOCK_RULE_PATH_BENEATH,&path_beneath_for_unix_socket,LANDLOCK_ADD_RULE_SOCK_STREAM|LANDLOCK_ADD_RULE_SOCK_DGRAM);For address families with only one socket type (ie tcp and udp), the socket family be implied, which keeps backward compatibility w/ the existing tcp access right.
This way, we don't have to make completely separate access rights for future socket families. So we could add a single access right for bluetooth, for instance, and distinguish which socket families we give it with the LANDLOCK_ADD_RULE_SOCK_* flags.
We'd have to track the
SOCK_(socket_type)for unix sockets as we gather access rights. But afaik unix sockets should be the only socket type that has to deal with tree traversal.Quoted 203 lines
* Hook into the path lookup in
unix_find_bsd()inaf_unix.c, using a LSM hook. Make policy decisions based on the new access rights
* Increment the Landlock ABI version.
* Minor test adaptions to keep the tests working.Cc Justin Suess <[email protected]>Cc Mickaël Salaün <[email protected]>Suggested-by Jann Horn <[email protected]>Signed-off-by Günther Noack <[email protected]>--- include/uapi/linux/landlock.h | 10 ++++++ security/landlock/access.h | 2 +- security/landlock/audit.c | 6 ++++ security/landlock/fs.c | 34 +++++++++++++++++++- security/landlock/limits.h | 2 +- security/landlock/syscalls.c | 2 +- tools/testing/selftests/landlock/base_test.c | 2 +- tools/testing/selftests/landlock/fs_test.c | 7 ++-- 8 files changed, 58 insertions(+), 7 deletions(-)include/uapi/linux/landlock.h +10
-0@@ -216,6 +216,13 @@ struct landlock_net_port_attr {* :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with * ``O_TRUNC``. This access right is available since the third version of the * Landlock ABI. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM: Connect to named +* :manpage:`unix(7)` ``SOCK_STREAM`` sockets. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM: Send messages to named +* :manpage:`unix(7)` ``SOCK_DGRAM`` sockets or connect to them using +* :manpage:`connect(2)`. +* - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET: Connect to named +* :manpage:`unix(7)` ``SOCK_SEQPACKET`` sockets. * * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as @@ -321,6 +328,9 @@ struct landlock_net_port_attr {#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM (1ULL << 16) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM (1ULL << 17) +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET (1ULL << 18)/* clang-format on */ /** security/landlock/access.h +1
-1@@ -34,7 +34,7 @@ LANDLOCK_ACCESS_FS_IOCTL_DEV)/* clang-format on */ -typedef u16 access_mask_t ; +typedef u32 access_mask_t ;/* Makes sure all filesystem access rights can be stored. */ static_assert (BITS_PER_TYPE (access_mask_t) >=LANDLOCK_NUM_ACCESS_FS );security/landlock/audit.c +6
-0@@ -37,6 +37,12 @@ static const char *const fs_access_strings[] = { [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer", [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate", [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM )] = + "fs.resolve_unix_stream", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM )] = + "fs.resolve_unix_dgram", + [BIT_INDEX (LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET )] = +"fs.resolve_unix_seqpacket" , };static_assert (ARRAY_SIZE (fs_access_strings) ==LANDLOCK_NUM_ACCESS_FS );security/landlock/fs.c +33
-1@@ -9,6 +9,7 @@* Copyright © 2023-2024 Google LLC */ +#include "linux/net.h" #include <asm/ioctls.h> #include <kunit/test.h> #include <linux/atomic.h> @@ -314,7 +315,10 @@ static struct landlock_object *get_inode_object(struct inode *const inode)LANDLOCK_ACCESS_FS_WRITE_FILE | \LANDLOCK_ACCESS_FS_READ_FILE | \LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) +LANDLOCK_ACCESS_FS_IOCTL_DEV | \ +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)/* clang-format on */ /* @@ -1588,6 +1592,33 @@ static int hook_path_truncate(const struct path *const path)return current_check_access_path (path,LANDLOCK_ACCESS_FS_TRUNCATE ); } +static int hook_unix_path_connect (const struct path *const path,int type, +int flags) +{ +access_mask_t access_request =0 ; + +/* Lookup for the purpose of saving coredumps is OK. */ +if (flags &SOCK_COREDUMP ) +return 0 ; + +switch (type) { +case SOCK_STREAM : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM ; +break ; +case SOCK_DGRAM : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM ; +break ; +case SOCK_SEQPACKET : + access_request =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET ; +break ; + } + +if (!access_request) +return 0 ; + +return current_check_access_path (path, access_request); +} +/* File hooks */ /** @@ -1872,6 +1903,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {LSM_HOOK_INIT (path_unlink, hook_path_unlink),LSM_HOOK_INIT (path_rmdir, hook_path_rmdir),LSM_HOOK_INIT (path_truncate, hook_path_truncate), +LSM_HOOK_INIT (unix_path_connect, hook_unix_path_connect),LSM_HOOK_INIT (file_alloc_security, hook_file_alloc_security), LSM_HOOK_INIT(file_open, hook_file_open),security/landlock/limits.h +1
-1@@ -19,7 +19,7 @@#define LANDLOCK_MAX_NUM_LAYERS 16#define LANDLOCK_MAX_NUM_RULES U32_MAX -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)security/landlock/syscalls.c +1
-1@@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {* Documentation/userspace-api/landlock.rst should be updated to reflect the * UAPI change. */ -const int landlock_abi_version =7 ; +const int landlock_abi_version =8 ;/** * sys_landlock_create_ruleset - Create a new ruleset tools/testing/selftests/landlock/base_test.c +1
-1@@ -76,7 +76,7 @@ TEST(abi_version)const struct landlock_ruleset_attr ruleset_attr = { .handled_access_fs =LANDLOCK_ACCESS_FS_READ_FILE , }; -ASSERT_EQ (7 ,landlock_create_ruleset (NULL ,0 , +ASSERT_EQ (8 ,landlock_create_ruleset (NULL ,0 ,LANDLOCK_CREATE_RULESET_VERSION )); ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,tools/testing/selftests/landlock/fs_test.c +5
-2@@ -575,9 +575,12 @@ TEST_F_FORK(layout1, inval) LANDLOCK_ACCESS_FS_WRITE_FILE | \ LANDLOCK_ACCESS_FS_READ_FILE | \ LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) + LANDLOCK_ACCESS_FS_IOCTL_DEV | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET) -#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV +#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET#define ACCESS_ALL ( \ ACCESS_FILE | \
PATCH v2 3/5 samples/landlock: Add support for named UNIX domain socket restrictions
In reply to Günther NoackThe access rights for UNIX domain socket lookups are grouped with the read-write rights in the sample tool. Rationale: In the general case, any operations are possible through a UNIX domain socket, including data-mutating operations.
--- samples/landlock/sandboxer.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
samples/landlock/sandboxer.c +16 -2
@@ -295,11 +295,14 @@ static bool check_ruleset_scope(const char *const env_var, LANDLOCK_ACCESS_FS_MAKE_SYM | \ LANDLOCK_ACCESS_FS_REFER | \ LANDLOCK_ACCESS_FS_TRUNCATE | \ - LANDLOCK_ACCESS_FS_IOCTL_DEV) + LANDLOCK_ACCESS_FS_IOCTL_DEV | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \ + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)/* clang-format on */ -#define LANDLOCK_ABI_LAST 7 +#define LANDLOCK_ABI_LAST 8#define XSTR (s) #s#define STR (s) XSTR(s)
@@ -444,6 +447,17 @@ int main(const int argc, char *const argv[], char *const *const envp)"provided by ABI version %d (instead of %d).\n" , LANDLOCK_ABI_LAST, abi); __attribute__((fallthrough)); +case 7 : +/* +* Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM, +* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM and +* LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET for ABI < 8 +*/ + ruleset_attr.handled_access_fs &= + ~(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | +LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET ); + __attribute__((fallthrough));case LANDLOCK_ABI_LAST :break ;default :
Signature · 1 line
2.52.0
Re: [PATCH v2 3/5] samples/landlock: Add support for named UNIX domain socket restrictions
Quoted 4 lines · Günther Noack
The access rights for UNIX domain socket lookups are grouped with the read-write rights in the sample tool. Rationale: In the general case, any operations are possible through a UNIX domain socket, including data-mutating operations.
Sorry, I missed a part of the discussion in V1, which was suggested by Tingmao Wang in [1]:
You are right, the new access rights should indeed become part of ACCESS_FILE in the sample tool. (When the sample tool is adding a rule for a non-directory, it only applies access rights that are also in ACCESS_FILE.)
Will add it in V3.
–Günther
PATCH v2 4/5 landlock/selftests: Test named UNIX domain socket restrictions
In reply to Günther Noack* Exercise the access rights for connect() and sendmsg() to named UNIX domain sockets, in various combinations.
* Extract common helpers from an existing IOCTL test that also uses pathname unix(7) sockets.
--- tools/testing/selftests/landlock/fs_test.c | 218 +++++++++++++++++++-- 1 file changed, 202 insertions(+), 16 deletions(-)
tools/testing/selftests/landlock/fs_test.c +202 -16
@@ -4360,30 +4360,61 @@ TEST_F_FORK(layout1, named_pipe_ioctl)ASSERT_EQ (child_pid,waitpid (child_pid,NULL ,0 )); } +/* +* set_up_named_unix_server - Create a pathname unix socket +* +* If the socket type is not SOCK_DGRAM, also invoke listen(2). +* +* Return: The listening FD - it is the caller responsibility to close it. +*/ +static int set_up_named_unix_server (struct __test_metadata *const _metadata, +int type,const char *const path) +{ +int fd; +struct sockaddr_un addr = { + .sun_family =AF_UNIX , + }; + + fd =socket (AF_UNIX , type,0 ); +ASSERT_LE (0 , fd); + +strncpy (addr.sun_path, path,sizeof (addr.sun_path)); +ASSERT_EQ (0 ,bind (fd, (struct sockaddr *)&addr,sizeof (addr))); + +if (type !=SOCK_DGRAM ) +ASSERT_EQ (0 ,listen (fd,10 /* qlen */ )); +return fd; +} + +/* +* test_connect_named_unix - connect to the given named UNIX socket +* +* Return: The errno from connect(), or 0 +*/ +static int test_connect_named_unix (int fd,const char *const path) +{ +struct sockaddr_un addr = { + .sun_family =AF_UNIX , + }; +strncpy (addr.sun_path, path,sizeof (addr.sun_path)); + +if (connect (fd, (struct sockaddr *)&addr,sizeof (addr)) ==-1 ) +return errno; +return 0 ; +} +/* For named UNIX domain sockets, no IOCTL restrictions apply. */ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl) {const char *const path = file1_s1d1;int srv_fd, cli_fd, ruleset_fd; -struct sockaddr_un srv_un = { - .sun_family =AF_UNIX , - }; -struct sockaddr_un cli_un = { - .sun_family =AF_UNIX , - };const struct landlock_ruleset_attr attr = { .handled_access_fs =LANDLOCK_ACCESS_FS_IOCTL_DEV , };/* Sets up a server */ ASSERT_EQ (0 ,unlink (path)); - srv_fd =socket (AF_UNIX ,SOCK_STREAM ,0 ); -ASSERT_LE (0 , srv_fd); - -strncpy (srv_un.sun_path, path,sizeof (srv_un.sun_path)); -ASSERT_EQ (0 ,bind (srv_fd, (struct sockaddr *)&srv_un,sizeof (srv_un))); - -ASSERT_EQ (0 ,listen (srv_fd,10 /* qlen */ )); + srv_fd =set_up_named_unix_server (_metadata,SOCK_STREAM , path);/* Enables Landlock. */ ruleset_fd =landlock_create_ruleset (&attr,sizeof (attr),0 );
@@ -4395,9 +4426,7 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl) cli_fd =socket (AF_UNIX ,SOCK_STREAM ,0 );ASSERT_LE (0 , cli_fd); -strncpy (cli_un.sun_path, path,sizeof (cli_un.sun_path)); -ASSERT_EQ (0 , -connect (cli_fd, (struct sockaddr *)&cli_un,sizeof (cli_un))); +ASSERT_EQ (0 ,test_connect_named_unix (cli_fd, path));/* FIONREAD and other IOCTLs should not be forbidden. */ EXPECT_EQ (0 ,test_fionread_ioctl (cli_fd));
@@ -4572,6 +4601,163 @@ TEST_F_FORK(ioctl, handle_file_access_file)ASSERT_EQ (0 ,close (file_fd)); } +/* clang-format off */ +FIXTURE (unix_socket) {}; + +FIXTURE_SETUP (unix_socket) {}; + +FIXTURE_TEARDOWN (unix_socket) {}; +/* clang-format on */ + +FIXTURE_VARIANT (unix_socket) +{ +const __u64 handled; +const __u64 allowed; +const int sock_type; +const int expected; +const bool use_sendto; +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, stream_handled_not_allowed) +{ +/* clang-format on */ + .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM, + .allowed = 0, + .sock_type = SOCK_STREAM, + .expected =EACCES , +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, stream_handled_and_allowed) +{ +/* clang-format on */ + .handled =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM , + .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM, + .sock_type = SOCK_STREAM, + .expected = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, dgram_handled_not_allowed) +{ +/* clang-format on */ + .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM, + .allowed = 0, + .sock_type = SOCK_DGRAM, + .expected =EACCES , +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, dgram_handled_and_allowed) +{ +/* clang-format on */ + .handled =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM , + .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM, + .sock_type = SOCK_DGRAM, + .expected = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, dgram_sendto_handled_not_allowed) +{ +/* clang-format on */ + .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM, + .allowed = 0, + .sock_type = SOCK_DGRAM, + .use_sendto = true, + .expected =EACCES , +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, dgram_sendto_handled_and_allowed) +{ +/* clang-format on */ + .handled =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM , + .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM, + .sock_type = SOCK_DGRAM, + .use_sendto = true, + .expected = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, seqpacket_handled_not_allowed) +{ +/* clang-format on */ + .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET, + .allowed = 0, + .sock_type = SOCK_SEQPACKET, + .expected =EACCES , +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD (unix_socket, seqpacket_handled_and_allowed) +{ +/* clang-format on */ + .handled =LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET , + .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET, + .sock_type = SOCK_SEQPACKET, + .expected = 0, +}; + +/* +* test_sendto_named_unix - sendto to the given named UNIX socket +* +* sendto() is equivalent to sendmsg() in this respect. +* +* Return: The errno from sendto(), or 0 +*/ +static int test_sendto_named_unix (int fd,const char *const path) +{ +static const char buf[] ="dummy" ; +struct sockaddr_un addr = { + .sun_family =AF_UNIX , + }; +strncpy (addr.sun_path, path,sizeof (addr.sun_path)); + +if (sendto (fd, buf,sizeof (buf),0 , (struct sockaddr *)&addr, +sizeof (addr)) ==-1 ) +return errno; +return 0 ; +} + +TEST_F_FORK(unix_socket, test) +{ +const char *const path ="sock" ; +int cli_fd, srv_fd, ruleset_fd, res; +const struct rule rules[] = { + { + .path = path, + .access = variant->allowed, + }, + {}, + }; + +/* Sets up a server */ + srv_fd =set_up_named_unix_server (_metadata, variant->sock_type, path); + +/* Enables Landlock. */ + ruleset_fd =create_ruleset (_metadata, variant->handled, rules); +ASSERT_LE (0 , ruleset_fd); +enforce_ruleset (_metadata, ruleset_fd); +ASSERT_EQ (0 ,close (ruleset_fd)); + +/* Sets up a client connection to it */ + cli_fd =socket (AF_UNIX , variant->sock_type,0 ); +ASSERT_LE (0 , cli_fd); + +/* Connecting or sendto to the Unix socket is denied. */ +if (variant->use_sendto) + res =test_sendto_named_unix (cli_fd, path); +else + res =test_connect_named_unix (cli_fd, path); +EXPECT_EQ (variant->expected, res); + +ASSERT_EQ (0 ,close (cli_fd)); +ASSERT_EQ (0 ,close (srv_fd)); +ASSERT_EQ (0 ,unlink (path)); +} +/* clang-format off */ FIXTURE (layout1_bind) {};/* clang-format on */
Signature · 1 line
2.52.0
PATCH v2 5/5 landlock: Document FS access rights for pathname UNIX sockets
In reply to Günther Noack--- Documentation/userspace-api/landlock.rst | 25 +++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
Documentation/userspace-api/landlock.rst +24 -1
@@ -77,7 +77,10 @@ to be explicit about the denied-by-default access rights. LANDLOCK_ACCESS_FS_MAKE_SYM | LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_TRUNCATE | - LANDLOCK_ACCESS_FS_IOCTL_DEV, + LANDLOCK_ACCESS_FS_IOCTL_DEV | + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET, .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP | LANDLOCK_ACCESS_NET_CONNECT_TCP,
@@ -127,6 +130,17 @@ version, and only use the available subset of access rights: /* Removes LANDLOCK_SCOPE_* for ABI < 6 */ ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPE_SIGNAL); + __attribute__((fallthrough)); + case 7: + /* + * Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM, + * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM and + * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET for ABI < 8 + */ + ruleset_attr.handled_access_fs &= + ~(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET); } This enables the creation of an inclusive ruleset that will contain our rules.
@@ -604,6 +618,15 @@ Landlock audit events with the ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``, sys_landlock_restrict_self(). See Documentation/admin-guide/LSM/landlock.rst for more details on audit. +Pathname UNIX sockets (ABI < 8) +------------------------------- + +Starting with the Landlock ABI version 8, it is possible to restrict +connections to pathname :manpage:`unix(7)` sockets using the new +``LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM``, +``LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM`` and +``LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET`` rights. + .. _kernel_support: Kernel support
Signature · 1 line
2.52.0
Mickaël Salaün
maintainerQuoted 47 lines · Günther Noack
Hello!
This patch set introduces a filesystem-based Landlock restriction mechanism for connecting to UNIX domain sockets (or addressing them with sendmsg(2)). It introduces a file system access right for each type of UNIX domain socket:
*
LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
*LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKETFor the connection-oriented
SOCK_STREAMandSOCK_SEQPACKETtype sockets, the access right makes the connect(2) operation fail with EACCES, if denied.SOCK_DGRAM-type UNIX sockets can be used both with connect(2), or by passing an explicit recipient address with every sendmsg(2) invocation. In the latter case, the Landlock check is done when an explicit recipient address is passed to sendmsg(2) and can make sendmsg(2) return EACCES. When UNIX datagram sockets are connected with connect(2), a fixed recipient address is associated with the socket and the check happens during connect(2) and may return EACCES.
## Motivation
Currently, landlocked processes can
connect()to named UNIX sockets through the BSD socket API described in unix(7), by invoking socket(2) followed by connect(2) with a suitable structsockname_unholding the socket's filename. This can come as a surprise for users (e.g. in [1]) and it can be used to escape a sandbox when a Unix service offers command execution (some scenarios were listed by Tingmao Wang in [2]).The original feature request is at [4].
## Alternatives and Related Work
### Alternative: Use existing LSM hooks
The existing hooks
security_unix_stream_connect(),security_unix_may_send()andsecurity_socket_connect()do not give access to the resolved file system path.Resolving the file system path again within Landlock would in my understanding produce a TOCTOU race, so making the decision based on the struct
sockaddr_uncontents is not an option.
Quoted 7 lines
It is tempting to use the struct path that the listening socket is bound to, which can be acquired through the existing hooks. Unfortunately, the listening socket may have been bound from within a different namespace, and it is therefore a path that can not actually be referenced by the sandboxed program at the time of constructing the Landlock policy. (More details are on the Github issue at [6] and on the LKML at [9]).
Please move (or duplicate) this rationale in the patch dedicated to the new hook. It helps patch review (and to understand commits when already merged).
Quoted 17 lines
### Related work: Scope Control for Pathname Unix Sockets
The motivation for this patch is the same as in Tingmao Wang's patch set for "scoped" control for pathname Unix sockets [2], originally proposed in the Github feature request [5].
In my reply to this patch set [3], I have discussed the differences between these two approaches. On the related discussions on Github
[4] and [5], there was consensus that the scope-based control is complimentary to the file system based control, but does not replace it. Mickael's opening remark on [5] says:Quoted 3 lines
This scoping would be complementary to #36 which would mainly be about allowing a sandboxed process to connect to a more privileged service (identified with a path).
Quoted 7 lines
## Open questions in V2
Seeking feedback on:
- Feedback on the LSM hook name would be appreciated. We realize that not all invocations of the LSM hook are related to connect(2) as the name suggests, but some also happen during sendmsg(2).
Renaming security_unix_path_connect() to security_unix_find() would look appropriate to me wrt the caller.
Quoted 4 lines
- Feedback on the structuring of the Landlock access rights, splitting them up by socket type. (Also naming; they are now consistently called "RESOLVE", but could be named "CONNECT" in the stream and seqpacket cases?)
I don't see use cases where differenciating the type of unix socket would be useful. LANDLOCK_ACCESS_FS_RESOLVE_UNIX would look good to me.
Tests should still cover all these types though.
What would be the inverse of "resolve" (i.e. to restrict the server side)? Would LANDLOCK_ACCESS_FS_MAKE_SOCK be enough?
Quoted 78 lines
## Credits
The feature was originally suggested by Jann Horn in [7].
Tingmao Wang and Demi Marie Obenour have taken the initiative to revive this discussion again in [1], [4] and [5] and Tingmao Wang has sent the patch set for the scoped access control for pathname Unix sockets [2].
Justin Suess has sent the patch for the LSM hook in [8].
Ryan Sullivan has started on an initial implementation and has brought up relevant discussion points on the Github issue at [4] that lead to the current approach.
[1] https://lore.kernel.org/landlock/[email protected]/
[2] Tingmao Wang's "Implemnet scope control for pathname Unix sockets"https://lore.kernel.org/all/[email protected]/[3] https://lore.kernel.org/all/[email protected]/
[4] Github issue for FS-based control for named Unix sockets:
https://github.com/landlock-lsm/linux/issues/36
[5] Github issue for scope-based restriction of named Unix sockets:
https://github.com/landlock-lsm/linux/issues/51
[6] https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
[7] https://lore.kernel.org/linux-security-module/CAG48ez3NvVnonOqKH4oRwRqbSOLO0p9djBqgvxVwn6gtGQBPcw@mail.gmail.com/
[8] Patch for the LSM hook:https://lore.kernel.org/all/[email protected]/[9] https://lore.kernel.org/all/[email protected]/
---
## Older versions of this patch set
V1: https://lore.kernel.org/all/[email protected]/
Changes in V2:
* Send Justin Suess's LSM hook patch together with the Landlock implementation
* LSM hook: Pass type and flags parameters to the hook, to make the access right more generally usable across LSMs, per suggestion from Paul Moore (Implemented by Justin)
* Split the access right into the three types of UNIX domain sockets:
SOCK_STREAM,SOCK_DGRAMandSOCK_SEQPACKET.
* selftests: More exhaustive tests.
* Removed a minor commit from V1 which adds a missing close(fd) to a test (it is already in the mic-next branch)Günther Noack (4):
landlock: Control pathname UNIX domain socket resolution by pathsamples/landlock: Add support for named UNIX domain socket
restrictions
landlock/selftests: Test named UNIX domain socket restrictions
landlock: Document FS access rights for pathname UNIX socketsJustin Suess (1):
lsm: Add hookunix_path_connectDocumentation/userspace-api/landlock.rst | 25 ++- include/linux/lsm_hook_defs.h | 4 + include/linux/security.h | 11 + include/uapi/linux/landlock.h | 10 + net/unix/af_unix.c | 9 + samples/landlock/sandboxer.c | 18 +- security/landlock/access.h | 2 +- security/landlock/audit.c | 6 + security/landlock/fs.c | 34 ++- security/landlock/limits.h | 2 +- security/landlock/syscalls.c | 2 +- security/security.c | 20 ++ tools/testing/selftests/landlock/base_test.c | 2 +- tools/testing/selftests/landlock/fs_test.c | 225 +++++++++++++++++-- 14 files changed, 344 insertions(+), 26 deletions(-)Signature · 1 line
2.52.0
Thanks for the review!
Quoted 22 lines · Mickaël Salaün
Quoted 13 lines · Günther Noack
## Alternatives and Related Work
### Alternative: Use existing LSM hooks
The existing hooks
security_unix_stream_connect(),security_unix_may_send()andsecurity_socket_connect()do not give access to the resolved file system path.Resolving the file system path again within Landlock would in my understanding produce a TOCTOU race, so making the decision based on the struct
sockaddr_uncontents is not an option.Quoted 7 lines
It is tempting to use the struct path that the listening socket is bound to, which can be acquired through the existing hooks. Unfortunately, the listening socket may have been bound from within a different namespace, and it is therefore a path that can not actually be referenced by the sandboxed program at the time of constructing the Landlock policy. (More details are on the Github issue at [6] and on the LKML at [9]).
Quoted 3 lines
Please move (or duplicate) this rationale in the patch dedicated to the new hook. It helps patch review (and to understand commits when already merged).
Justin, would you like to look into this?
Please feel free to copy the wording.
Quoted 18 lines
Quoted 18 lines
### Related work: Scope Control for Pathname Unix Sockets
The motivation for this patch is the same as in Tingmao Wang's patch set for "scoped" control for pathname Unix sockets [2], originally proposed in the Github feature request [5].
In my reply to this patch set [3], I have discussed the differences between these two approaches. On the related discussions on Github
[4] and [5], there was consensus that the scope-based control is complimentary to the file system based control, but does not replace it. Mickael's opening remark on [5] says:Quoted 3 lines
This scoping would be complementary to #36 which would mainly be about allowing a sandboxed process to connect to a more privileged service (identified with a path).
## Open questions in V2
Quoted 8 lines
Quoted 5 lines
Seeking feedback on:
- Feedback on the LSM hook name would be appreciated. We realize that not all invocations of the LSM hook are related to connect(2) as the name suggests, but some also happen during sendmsg(2).
Renaming
security_unix_path_connect()tosecurity_unix_find()would look appropriate to me wrt the caller.
Justin, this is also on your commit. (I find security_unix_find() and security_unix_resolve() equally acceptable options.)
Quoted 7 lines
Quoted 4 lines
- Feedback on the structuring of the Landlock access rights, splitting them up by socket type. (Also naming; they are now consistently called "RESOLVE", but could be named "CONNECT" in the stream and seqpacket cases?)
I don't see use cases where differenciating the type of unix socket would be useful.
LANDLOCK_ACCESS_FS_RESOLVE_UNIXwould look good to me.
I did it mostly because it seemed consistent with the TCP and (soon) UDP controls, which are also controls specific to the socket type and not just the address family. But I agree that the granularity is likely not needed here. I can change it back for v3 and rename it to LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
Quoted 2 lines
What would be the inverse of "resolve" (i.e. to restrict the server side)? Would
LANDLOCK_ACCESS_FS_MAKE_SOCKbe enough?
Yes, that would be enough. My reasoning is as follows:
The server-side operation that is related to associating the service with a given file system name is bind(2), and that is restrictable in that case using LANDLOCK_ACCESS_FS_MAKE_SOCK.
Also, to my delight (and other than in TCP), listening on an unbound socket does not work (see unix_listen() in af_unix.c):
if (!READ_ONCE(u->addr))
goto out; /* No listens on an unbound socket */
(You can get it to "autobind" during an explicit bind() or a connect() call, but that creates an abstract UNIX address. (Documented in socket(7) and implemented in unix_autobind() in af_unix.c))
–Günther
Quoted 23 lines · Günther Noack
Thanks for the review!
Quoted 20 lines · Mickaël Salaün
Quoted 19 lines · Günther Noack
## Alternatives and Related Work
### Alternative: Use existing LSM hooks
The existing hooks
security_unix_stream_connect(),security_unix_may_send()andsecurity_socket_connect()do not give access to the resolved file system path.Resolving the file system path again within Landlock would in my understanding produce a TOCTOU race, so making the decision based on the struct
sockaddr_uncontents is not an option.It is tempting to use the struct path that the listening socket is bound to, which can be acquired through the existing hooks. Unfortunately, the listening socket may have been bound from within a different namespace, and it is therefore a path that can not actually be referenced by the sandboxed program at the time of constructing the Landlock policy. (More details are on the Github issue at [6] and on the LKML at [9]).
Quoted 5 lines
Quoted 3 lines
Please move (or duplicate) this rationale in the patch dedicated to the new hook. It helps patch review (and to understand commits when already merged).
Justin, would you like to look into this?
Please feel free to copy the wording.
No problem.
It's quite long, so would it make sense in the notes? Instead of directly in the commit message?
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
### Related work: Scope Control for Pathname Unix Sockets
The motivation for this patch is the same as in Tingmao Wang's patch set for "scoped" control for pathname Unix sockets [2], originally proposed in the Github feature request [5].
In my reply to this patch set [3], I have discussed the differences between these two approaches. On the related discussions on Github
[4] and [5], there was consensus that the scope-based control is complimentary to the file system based control, but does not replace it. Mickael's opening remark on [5] says:Quoted 3 lines
This scoping would be complementary to #36 which would mainly be about allowing a sandboxed process to connect to a more privileged service (identified with a path).
## Open questions in V2
Seeking feedback on:
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
- Feedback on the LSM hook name would be appreciated. We realize that not all invocations of the LSM hook are related to connect(2) as the name suggests, but some also happen during sendmsg(2).
Renaming
security_unix_path_connect()tosecurity_unix_find()would look appropriate to me wrt the caller.Justin, this is also on your commit. (I find
security_unix_find()andsecurity_unix_resolve()equally acceptable options.)
security_unix_find works for me, and seems to better match the hook location. I'll send an updated commit.
Quoted 35 lines
Quoted 8 lines
Quoted 6 lines
- Feedback on the structuring of the Landlock access rights, splitting them up by socket type. (Also naming; they are now consistently called "RESOLVE", but could be named "CONNECT" in the stream and seqpacket cases?)
I don't see use cases where differenciating the type of unix socket would be useful.
LANDLOCK_ACCESS_FS_RESOLVE_UNIXwould look good to me.I did it mostly because it seemed consistent with the TCP and (soon) UDP controls, which are also controls specific to the socket type and not just the address family. But I agree that the granularity is likely not needed here. I can change it back for v3 and rename it to
LANDLOCK_ACCESS_FS_RESOLVE_UNIX.Quoted 2 lines
What would be the inverse of "resolve" (i.e. to restrict the server side)? Would
LANDLOCK_ACCESS_FS_MAKE_SOCKbe enough?Yes, that would be enough. My reasoning is as follows:
The server-side operation that is related to associating the service with a given file system name is bind(2), and that is restrictable in that case using
LANDLOCK_ACCESS_FS_MAKE_SOCK.Also, to my delight (and other than in TCP), listening on an unbound socket does not work (see
unix_listen()inaf_unix.c):if (!READ_ONCE(u->addr))
goto out; /* No listens on an unbound socket */(You can get it to "autobind" during an explicit
bind()or aconnect()call, but that creates an abstract UNIX address. (Documented in socket(7) and implemented inunix_autobind()inaf_unix.c))–Günther
Hello!
Quoted 25 lines · Justin Suess
Quoted 24 lines · Günther Noack
Quoted 23 lines · Mickaël Salaün
Quoted 12 lines · Günther Noack
## Alternatives and Related Work
### Alternative: Use existing LSM hooks
The existing hooks
security_unix_stream_connect(),security_unix_may_send()andsecurity_socket_connect()do not give access to the resolved file system path.Resolving the file system path again within Landlock would in my understanding produce a TOCTOU race, so making the decision based on the struct
sockaddr_uncontents is not an option.Quoted 7 lines
It is tempting to use the struct path that the listening socket is bound to, which can be acquired through the existing hooks. Unfortunately, the listening socket may have been bound from within a different namespace, and it is therefore a path that can not actually be referenced by the sandboxed program at the time of constructing the Landlock policy. (More details are on the Github issue at [6] and on the LKML at [9]).
Please move (or duplicate) this rationale in the patch dedicated to the new hook. It helps patch review (and to understand commits when already merged).
Quoted 6 lines
Quoted 2 lines
Justin, would you like to look into this?
Please feel free to copy the wording.No problem.
It's quite long, so would it make sense in the notes? Instead of directly in the commit message?
I think including it in the commit message is what Mickaël meant here. The quoted email above is already from the cover letter (which I assume you meant by "notes"?). IMHO, the considerations that are specific to the LSM hook are OK to put on the commit that introduces it, even if they are a bit longer. That way, a summary of the tradeoffs also makes its way into the overall commit history (unlike the cover letter).
FWIW, commit messages with long descriptions are not unheard of, e.g. commit ee6a44da3c87 ("tty: Require CAP_SYS_ADMIN for all usages of TIOCL_SELMOUSEREPORT"), which I submitted a while back.
For reference, the official guidance on commit messages is
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
Quoted 11 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
Seeking feedback on:
- Feedback on the LSM hook name would be appreciated. We realize that not all invocations of the LSM hook are related to connect(2) as the name suggests, but some also happen during sendmsg(2).
Renaming
security_unix_path_connect()tosecurity_unix_find()would look appropriate to me wrt the caller.Justin, this is also on your commit. (I find
security_unix_find()andsecurity_unix_resolve()equally acceptable options.)
security_unix_findworks for me, and seems to better match the hook location. I'll send an updated commit.
Thanks! Please feel free to ping me, I'd be ready to send an updated v3.
–Günther