Re: [RFC PATCH 0/1] lsm: Add hook unix_path_connect

[Justin Suess <utilityemal77@gmail.com>]

On 1/1/26 07:13, Günther Noack wrote:
> On Wed, Dec 31, 2025 at 04:33:14PM -0500, Justin Suess wrote:
>> 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.
>>
>> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
>> Cc: Günther Noack <gnoack3000@gmail.com>
>> ---
>>  include/linux/lsm_hook_defs.h |  1 +
>>  include/linux/security.h      |  6 ++++++
>>  net/unix/af_unix.c            |  8 ++++++++
>>  security/security.c           | 16 ++++++++++++++++
>>  4 files changed, 31 insertions(+)
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index 8c42b4bde09c..a42d1aaf3b8a 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -318,6 +318,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
>>  #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>>  
>>  #ifdef CONFIG_SECURITY_NETWORK
>> +LSM_HOOK(int, 0, unix_path_connect, const struct path *path)
>
> You are placing this guarded by CONFIG_SECURITY_NETWORK, but there is
> also CONFIG_SECURITY_PATH.  Should it be guarded by both?

Agreed. I've moved it to a separate #if block with both
CONFIG_SECURITY_NETWORK and CONFIG_SECURITY_PATH for this and the other
places it was needed.

>
>
>
>>  LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
>>  	 struct sock *newsk)
>>  LSM_HOOK(int, 0, unix_may_send, struct socket *sock, struct socket *other)
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 83a646d72f6f..ab66f22f7e5a 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -1638,6 +1638,7 @@ static inline int security_watch_key(struct key *key)
>>  
>>  #ifdef CONFIG_SECURITY_NETWORK
>>  
>> +int security_unix_path_connect(const struct path *path);
>>  int security_netlink_send(struct sock *sk, struct sk_buff *skb);
>>  int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk);
>>  int security_unix_may_send(struct socket *sock,  struct socket *other);
>> @@ -1699,6 +1700,11 @@ static inline int security_netlink_send(struct sock *sk, struct sk_buff *skb)
>>  	return 0;
>>  }
>>  
>> +static inline int security_unix_path_connect(const struct path *path)
>> +{
>> +	return 0;
>> +}
>> +
>>  static inline int security_unix_stream_connect(struct sock *sock,
>>  					       struct sock *other,
>>  					       struct sock *newsk)
>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>> index 55cdebfa0da0..af1a6083a69b 100644
>> --- a/net/unix/af_unix.c
>> +++ b/net/unix/af_unix.c
>> @@ -1226,6 +1226,14 @@ 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);
>> +	if (err)
>> +		goto path_put;
>
> In this place, the hook call is done also for the coredump socket.
>
> The coredump socket is a system-wide setting, and it feels weird to me
> that unprivileged processes should be able to inhibit that connection?

No I don't think they should be able to. Does this look better?

It also fixes overwriting the the error code when the hook returns.

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 55cdebfa0da0..397687e2d87f 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1226,6 +1226,18 @@ 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.
+        * We intentionally forgo the ability to restrict SOCK_COREDUMP.
+        */
+       if (!(flags & SOCK_COREDUMP)) {
+               err = security_unix_path_connect(&path);
+               if (err)
+                       goto path_put;
+               err = -ECONNREFUSED;
+       }
+
        sk = unix_find_socket_byinode(inode);
        if (!sk)
                goto path_put;

>
>
>> +
>>  	sk = unix_find_socket_byinode(inode);
>>  	if (!sk)
>>  		goto path_put;
>> diff --git a/security/security.c b/security/security.c
>> index 31a688650601..17af5d0ddf28 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -4047,6 +4047,22 @@ int security_unix_stream_connect(struct sock *sock, struct sock *other,
>>  }
>>  EXPORT_SYMBOL(security_unix_stream_connect);
>>  
>> +/*
>> + * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
>> + * @path: Path of the socket being connected to
>              ^
> mega-nit: lowercase for consistency
Gotcha.
>
>
>> + *
>> + * This hook is called to check permissions before connecting to a named
>> + * AF_UNIX socket. This is necessary because it was not possible to check the
>> + * VFS inode of the target socket before the connection is made.
>
> I'd drop the last sentence; the defense why this is necessary can go
> in the commit message, and once we have a call-site for the hook,
> someone browsing the kernel code can look up what it is used for.
Sounds good to me.
>
>
>> + *
>> + * Return: Returns 0 if permission is granted.
>> + */
>> +int security_unix_path_connect(const struct path *path)
>> +{
>> +	return call_int_hook(unix_path_connect, path);
>> +}
>> +EXPORT_SYMBOL(security_unix_path_connect);
>> +
>>  /**
>>   * security_unix_may_send() - Check if AF_UNIX socket can send datagrams
>>   * @sock: originating sock
>> -- 
>> 2.51.0
>>