[RFC PATCH] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[RFC PATCH] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[Casey Schaufler]

We recently introduced system calls to access process attributes that
are used by Linux Security Modules (LSM). An important aspect of these
system calls is that they provide the LSM attribute data in a format
that identifies the LSM to which the data applies. Another aspect is that
it can be used to provide multiple instances of the attribute for the
case where more than one LSM supplies the attribute.

We wish to take advantage of this format for data about network peers.
The existing mechanism, SO_PEERSEC, provides peer security data as a
text string. This is sufficient when the LSM providing the information
is known by the user of SO_PEERSEC, and there is only one LSM providing
the information. It fails, however, if the user does not know which
LSM is providing the information.

Discussions about extending SO_PEERSEC to accomodate either the new
format or some other encoding scheme invariably lead to the conclusion
that doing so would lead to tears. Hence, we introduce SO_PEERCONTEXT
which uses the same API data as the LSM system calls.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 arch/alpha/include/uapi/asm/socket.h  |  1 +
 arch/mips/include/uapi/asm/socket.h   |  1 +
 arch/parisc/include/uapi/asm/socket.h |  1 +
 arch/sparc/include/uapi/asm/socket.h  |  1 +
 include/linux/lsm_hook_defs.h         |  2 +
 include/linux/security.h              | 18 ++++++++
 include/uapi/asm-generic/socket.h     |  1 +
 net/core/sock.c                       |  4 ++
 security/apparmor/lsm.c               | 39 ++++++++++++++++
 security/security.c                   | 86 +++++++++++++++++++++++++++++++++++
 security/selinux/hooks.c              | 35 ++++++++++++++
 security/smack/smack_lsm.c            | 25 ++++++++++
 12 files changed, 214 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index e94f621903fe..e49ed0a765ce 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -139,6 +139,7 @@
 
 #define SO_PASSPIDFD		76
 #define SO_PEERPIDFD		77
+#define SO_PEERCONTEXT		78
 
 #if !defined(__KERNEL__)
 
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 60ebaed28a4c..e647f065b3c9 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -150,6 +150,7 @@
 
 #define SO_PASSPIDFD		76
 #define SO_PEERPIDFD		77
+#define SO_PEERCONTEXT		78
 
 #if !defined(__KERNEL__)
 
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index be264c2b1a11..4f48bef863bc 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -131,6 +131,7 @@
 
 #define SO_PASSPIDFD		0x404A
 #define SO_PEERPIDFD		0x404B
+#define SO_PEERCONTEXT		0x404C
 
 #if !defined(__KERNEL__)
 
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 682da3714686..b54c56056323 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -132,6 +132,7 @@
 
 #define SO_PASSPIDFD             0x0055
 #define SO_PEERPIDFD             0x0056
+#define SO_PEERCONTEXT           0x0057
 
 #if !defined(__KERNEL__)
 
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 27e6384ec779..d923acb40c20 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -336,6 +336,8 @@ LSM_HOOK(int, 0, socket_shutdown, struct socket *sock, int how)
 LSM_HOOK(int, 0, socket_sock_rcv_skb, struct sock *sk, struct sk_buff *skb)
 LSM_HOOK(int, -ENOPROTOOPT, socket_getpeersec_stream, struct socket *sock,
 	 sockptr_t optval, sockptr_t optlen, unsigned int len)
+LSM_HOOK(int, -ENOPROTOOPT, socket_getpeerctx_stream, struct socket *sock,
+	 sockptr_t optval, sockptr_t optlen, unsigned int len)
 LSM_HOOK(int, -ENOPROTOOPT, socket_getpeersec_dgram, struct socket *sock,
 	 struct sk_buff *skb, u32 *secid)
 LSM_HOOK(int, 0, sk_alloc_security, struct sock *sk, int family, gfp_t priority)
diff --git a/include/linux/security.h b/include/linux/security.h
index a54543ded9fc..d4f449b7deea 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -563,6 +563,8 @@ int security_inode_getsecctx(struct inode *inode, struct lsmcontext *cp);
 int security_locked_down(enum lockdown_reason what);
 int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
 		      void *val, size_t val_len, u64 id, u64 flags);
+int lsm_fill_socket_ctx(sockptr_t optval, sockptr_t optlen, void *val,
+			size_t val_len, u64 id, u64 flags);
 #else /* CONFIG_SECURITY */
 
 static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -1552,6 +1554,12 @@ static inline int lsm_fill_user_ctx(struct lsm_ctx __user *uctx,
 {
 	return -EOPNOTSUPP;
 }
+static inline int lsm_fill_socket_ctx(sockptr_t optval, sockptr_t optlen,
+				      void *val, size_t val_len, u64 id,
+				      u64 flags)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_SECURITY */
 
 #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
@@ -1599,6 +1607,8 @@ int security_socket_shutdown(struct socket *sock, int how);
 int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb);
 int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
 				      sockptr_t optlen, unsigned int len);
+int security_socket_getpeerctx_stream(struct socket *sock, sockptr_t optval,
+				      sockptr_t optlen, unsigned int len);
 int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid);
 int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
 void security_sk_free(struct sock *sk);
@@ -1744,6 +1754,14 @@ static inline int security_socket_getpeersec_stream(struct socket *sock,
 	return -ENOPROTOOPT;
 }
 
+static inline int security_socket_getpeerctx_stream(struct socket *sock,
+						    sockptr_t optval,
+						    sockptr_t optlen,
+						    unsigned int len)
+{
+	return -ENOPROTOOPT;
+}
+
 static inline int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
 {
 	return -ENOPROTOOPT;
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 8ce8a39a1e5f..e0166ff53670 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -134,6 +134,7 @@
 
 #define SO_PASSPIDFD		76
 #define SO_PEERPIDFD		77
+#define SO_PEERCONTEXT		78
 
 #if !defined(__KERNEL__)
 
diff --git a/net/core/sock.c b/net/core/sock.c
index 0963689a5950..251346eccfa5 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1849,6 +1849,10 @@ int sk_getsockopt(struct sock *sk, int level, int optname,
 		return security_socket_getpeersec_stream(sock,
 							 optval, optlen, len);
 
+	case SO_PEERCONTEXT:
+		return security_socket_getpeerctx_stream(sock,
+							 optval, optlen, len);
+
 	case SO_MARK:
 		v.val = READ_ONCE(sk->sk_mark);
 		break;
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c478493f3eaf..66d4ecd6afcb 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1357,6 +1357,43 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock,
 	return error;
 }
 
+/**
+ * apparmor_socket_getpeerctx_stream - get security context of peer
+ * @sock: socket that we are trying to get the peer context of
+ * @optval: output - buffer to copy peer name to
+ * @optlen: output - size of copied name in @optval
+ * @len: size of @optval buffer
+ * Returns: 0 on success, -errno of failure
+ *
+ * Note: for tcp only valid if using ipsec or cipso on lan
+ */
+static int apparmor_socket_getpeerctx_stream(struct socket *sock,
+					     sockptr_t optval, sockptr_t optlen,
+					     unsigned int len)
+{
+	char *name = NULL;
+	int slen, error = 0;
+	struct aa_label *label;
+	struct aa_label *peer;
+
+	label = begin_current_label_crit_section();
+	peer = sk_peer_label(sock->sk);
+	if (IS_ERR(peer)) {
+		error = PTR_ERR(peer);
+		goto done;
+	}
+	slen = aa_label_asxprint(&name, labels_ns(label), peer,
+				 FLAG_SHOW_MODE | FLAG_VIEW_SUBNS |
+				 FLAG_HIDDEN_UNCONFINED, GFP_KERNEL);
+
+	error = lsm_fill_socket_ctx(optval, optlen, name, slen,
+				    LSM_ID_APPARMOR, 0);
+done:
+	end_current_label_crit_section(label);
+	kfree(name);
+	return error;
+}
+
 /**
  * apparmor_sock_graft - Initialize newly created socket
  * @sk: child sock
@@ -1466,6 +1503,8 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
 #endif
 	LSM_HOOK_INIT(socket_getpeersec_stream,
 		      apparmor_socket_getpeersec_stream),
+	LSM_HOOK_INIT(socket_getpeerctx_stream,
+		      apparmor_socket_getpeerctx_stream),
 	LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
 #ifdef CONFIG_NETWORK_SECMARK
 	LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
diff --git a/security/security.c b/security/security.c
index e387614cb054..fd4919c28e8f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -874,6 +874,64 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
 	return rc;
 }
 
+/**
+ * lsm_fill_socket_ctx - Fill a socket lsm_ctx structure
+ * @optval: a socket LSM context to be filled
+ * @optlen: uctx size
+ * @val: the new LSM context value
+ * @val_len: the size of the new LSM context value
+ * @id: LSM id
+ * @flags: LSM defined flags
+ *
+ * Fill all of the fields in a lsm_ctx structure.  If @optval is NULL
+ * simply calculate the required size to output via @optlen and return
+ * success.
+ *
+ * Returns 0 on success, -E2BIG if userspace buffer is not large enough,
+ * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated.
+ */
+int lsm_fill_socket_ctx(sockptr_t optval, sockptr_t optlen, void *val,
+			size_t val_len, u64 id, u64 flags)
+{
+	struct lsm_ctx *nctx = NULL;
+	unsigned int nctx_len;
+	int loptlen;
+	int rc = 0;
+
+	if (copy_from_sockptr(&loptlen, optlen, sizeof(int)))
+		return -EFAULT;
+
+	nctx_len = ALIGN(struct_size(nctx, ctx, val_len), sizeof(void *));
+	if (nctx_len > loptlen && !sockptr_is_null(optval))
+		rc = -E2BIG;
+
+	/* no buffer - return success/0 and set @uctx_len to the req size */
+	if (sockptr_is_null(optval) || rc)
+		goto out;
+
+	nctx = kzalloc(nctx_len, GFP_KERNEL);
+	if (!nctx) {
+		rc = -ENOMEM;
+		goto out;
+	}
+	nctx->id = id;
+	nctx->flags = flags;
+	nctx->len = nctx_len;
+	nctx->ctx_len = val_len;
+	memcpy(nctx->ctx, val, val_len);
+
+	if (copy_to_sockptr(optval, nctx, nctx_len))
+		rc = -EFAULT;
+
+	kfree(nctx);
+out:
+	if (copy_to_sockptr(optlen, &nctx_len, sizeof(int)))
+		rc = -EFAULT;
+
+	return rc;
+}
+
+
 /*
  * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
  * can be accessed with:
@@ -4743,6 +4801,34 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
 	return LSM_RET_DEFAULT(socket_getpeersec_stream);
 }
 
+/**
+ * security_socket_getpeerctx_stream() - Get the remote peer label
+ * @sock: socket
+ * @optval: destination buffer
+ * @optlen: size of peer label copied into the buffer
+ * @len: maximum size of the destination buffer
+ *
+ * This hook allows the security module to provide peer socket security state
+ * for unix or connected tcp sockets to userspace via getsockopt
+ * SO_GETPEERCONTEXT.  For tcp sockets this can be meaningful if the socket
+ * is associated with an ipsec SA.
+ *
+ * Return: Returns 0 if all is well, otherwise, typical getsockopt return
+ *         values.
+ */
+int security_socket_getpeerctx_stream(struct socket *sock, sockptr_t optval,
+				      sockptr_t optlen, unsigned int len)
+{
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeerctx_stream,
+			     list)
+		return hp->hook.socket_getpeerctx_stream(sock, optval, optlen,
+							 len);
+
+	return LSM_RET_DEFAULT(socket_getpeerctx_stream);
+}
+
 /**
  * security_socket_getpeersec_dgram() - Get the remote peer label
  * @sock: socket
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 83d30579e568..3c34cbc2b197 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5221,6 +5221,39 @@ static int selinux_socket_getpeersec_stream(struct socket *sock,
 	return err;
 }
 
+static int selinux_socket_getpeerctx_stream(struct socket *sock,
+					    sockptr_t optval, sockptr_t optlen,
+					    unsigned int len)
+{
+	int err = 0;
+	char *scontext = NULL;
+	u32 scontext_len;
+	struct sk_security_struct *sksec = selinux_sock(sock->sk);
+	u32 peer_sid = SECSID_NULL;
+
+	if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
+	    sksec->sclass == SECCLASS_TCP_SOCKET ||
+	    sksec->sclass == SECCLASS_SCTP_SOCKET)
+		peer_sid = sksec->peer_sid;
+	if (peer_sid == SECSID_NULL)
+		return -ENOPROTOOPT;
+
+	err = security_sid_to_context(peer_sid, &scontext,
+				      &scontext_len);
+	if (err)
+		return err;
+	if (scontext_len > len) {
+		err = -ERANGE;
+		goto out_len;
+	}
+
+	err = lsm_fill_socket_ctx(optval, optlen, scontext, scontext_len,
+				  LSM_ID_SELINUX, 0);
+out_len:
+	kfree(scontext);
+	return err;
+}
+
 static int selinux_socket_getpeersec_dgram(struct socket *sock,
 					   struct sk_buff *skb, u32 *secid)
 {
@@ -7345,6 +7378,8 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_sock_rcv_skb, selinux_socket_sock_rcv_skb),
 	LSM_HOOK_INIT(socket_getpeersec_stream,
 			selinux_socket_getpeersec_stream),
+	LSM_HOOK_INIT(socket_getpeerctx_stream,
+			selinux_socket_getpeerctx_stream),
 	LSM_HOOK_INIT(socket_getpeersec_dgram, selinux_socket_getpeersec_dgram),
 	LSM_HOOK_INIT(sk_free_security, selinux_sk_free_security),
 	LSM_HOOK_INIT(sk_clone_security, selinux_sk_clone_security),
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 3380a9e91fb8..dd0ea4da7eeb 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4297,6 +4297,30 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
 	return rc;
 }
 
+/**
+ * smack_socket_getpeerctx_stream - pull in packet label
+ * @sock: the socket
+ * @optval: user's destination
+ * @optlen: size thereof
+ * @len: max thereof
+ *
+ * returns zero on success, an error code otherwise
+ */
+static int smack_socket_getpeerctx_stream(struct socket *sock,
+					  sockptr_t optval, sockptr_t optlen,
+					  unsigned int len)
+{
+	struct socket_smack *ssp;
+	size_t slen;
+	char *rcp = "";
+
+	ssp = smack_sock(sock->sk);
+	if (ssp->smk_packet)
+		rcp = ssp->smk_packet->smk_known;
+	slen = strlen(rcp) + 1;
+
+	return lsm_fill_socket_ctx(optval, optlen, rcp, slen, LSM_ID_SMACK, 0);
+}
 
 /**
  * smack_socket_getpeersec_dgram - pull in packet label
@@ -5206,6 +5230,7 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_sendmsg, smack_socket_sendmsg),
 	LSM_HOOK_INIT(socket_sock_rcv_skb, smack_socket_sock_rcv_skb),
 	LSM_HOOK_INIT(socket_getpeersec_stream, smack_socket_getpeersec_stream),
+	LSM_HOOK_INIT(socket_getpeerctx_stream, smack_socket_getpeerctx_stream),
 	LSM_HOOK_INIT(socket_getpeersec_dgram, smack_socket_getpeersec_dgram),
 	LSM_HOOK_INIT(sk_alloc_security, smack_sk_alloc_security),
 #ifdef SMACK_IPV6_PORT_LABELING

Re: [PATCH RFC] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[Paul Moore]

On May 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> 
> We recently introduced system calls to access process attributes that
> are used by Linux Security Modules (LSM). An important aspect of these
> system calls is that they provide the LSM attribute data in a format
> that identifies the LSM to which the data applies. Another aspect is that
> it can be used to provide multiple instances of the attribute for the
> case where more than one LSM supplies the attribute.
> 
> We wish to take advantage of this format for data about network peers.
> The existing mechanism, SO_PEERSEC, provides peer security data as a
> text string. This is sufficient when the LSM providing the information
> is known by the user of SO_PEERSEC, and there is only one LSM providing
> the information. It fails, however, if the user does not know which
> LSM is providing the information.
> 
> Discussions about extending SO_PEERSEC to accomodate either the new

Spelling nitpick -> "accommodate" :)

> format or some other encoding scheme invariably lead to the conclusion
> that doing so would lead to tears. Hence, we introduce SO_PEERCONTEXT
> which uses the same API data as the LSM system calls.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  arch/alpha/include/uapi/asm/socket.h  |  1 +
>  arch/mips/include/uapi/asm/socket.h   |  1 +
>  arch/parisc/include/uapi/asm/socket.h |  1 +
>  arch/sparc/include/uapi/asm/socket.h  |  1 +
>  include/linux/lsm_hook_defs.h         |  2 +
>  include/linux/security.h              | 18 ++++++++
>  include/uapi/asm-generic/socket.h     |  1 +
>  net/core/sock.c                       |  4 ++
>  security/apparmor/lsm.c               | 39 ++++++++++++++++
>  security/security.c                   | 86 +++++++++++++++++++++++++++++++++++
>  security/selinux/hooks.c              | 35 ++++++++++++++
>  security/smack/smack_lsm.c            | 25 ++++++++++
>  12 files changed, 214 insertions(+)

...

> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index 8ce8a39a1e5f..e0166ff53670 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -134,6 +134,7 @@
>  
>  #define SO_PASSPIDFD		76
>  #define SO_PEERPIDFD		77
> +#define SO_PEERCONTEXT		78

Bikeshed time ... how about SO_PEERLSMCTX since we are returning a
lsm_ctx struct?

> diff --git a/security/security.c b/security/security.c
> index e387614cb054..fd4919c28e8f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -874,6 +874,64 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
>  	return rc;
>  }
>  
> +/**
> + * lsm_fill_socket_ctx - Fill a socket lsm_ctx structure
> + * @optval: a socket LSM context to be filled
> + * @optlen: uctx size

"@optlen: @optval size"

> + * @val: the new LSM context value
> + * @val_len: the size of the new LSM context value
> + * @id: LSM id
> + * @flags: LSM defined flags
> + *
> + * Fill all of the fields in a lsm_ctx structure.  If @optval is NULL
> + * simply calculate the required size to output via @optlen and return
> + * success.
> + *
> + * Returns 0 on success, -E2BIG if userspace buffer is not large enough,
> + * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated.
> + */
> +int lsm_fill_socket_ctx(sockptr_t optval, sockptr_t optlen, void *val,
> +			size_t val_len, u64 id, u64 flags)
> +{
> +	struct lsm_ctx *nctx = NULL;
> +	unsigned int nctx_len;
> +	int loptlen;

u32?

> +	int rc = 0;
> +
> +	if (copy_from_sockptr(&loptlen, optlen, sizeof(int)))
> +		return -EFAULT;

It seems the current guidance prefers copy_safe_from_sockptr(), see
the note in include/linux/sockptr.h. 

> +	nctx_len = ALIGN(struct_size(nctx, ctx, val_len), sizeof(void *));
> +	if (nctx_len > loptlen && !sockptr_is_null(optval))
> +		rc = -E2BIG;

Why do we care if @optval is NULL or not?  We are in a -E2BIG state,
we're not copying anything into @optval anyway.  In fact, why are we
doing the @rc check below?  Do it here like we do in lsm_fill_user_ctx().

  if (nctx_len > loptlen) {
    rc = -E2BIG;
    goto out;
  }

> +	/* no buffer - return success/0 and set @uctx_len to the req size */

"... set @opt_len ... "

> +	if (sockptr_is_null(optval) || rc)
> +		goto out;

Do the @rc check above, not here.

> +	nctx = kzalloc(nctx_len, GFP_KERNEL);
> +	if (!nctx) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +	nctx->id = id;
> +	nctx->flags = flags;
> +	nctx->len = nctx_len;
> +	nctx->ctx_len = val_len;
> +	memcpy(nctx->ctx, val, val_len);
> +
> +	if (copy_to_sockptr(optval, nctx, nctx_len))
> +		rc = -EFAULT;

This is always going to copy to the start of @optval which means we
are going to keep overwriting previous values in the multi-LSM case.
I think we likely want copy_to_sockptr_offset(), or similar.  See my
comment in security_socket_getpeerctx_stream().

> +	kfree(nctx);
> +out:
> +	if (copy_to_sockptr(optlen, &nctx_len, sizeof(int)))
> +		rc = -EFAULT;
> +
> +	return rc;
> +}
> +
> +
>  /*
>   * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
>   * can be accessed with:
> @@ -4743,6 +4801,34 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
>  	return LSM_RET_DEFAULT(socket_getpeersec_stream);
>  }
>  
> +/**
> + * security_socket_getpeerctx_stream() - Get the remote peer label
> + * @sock: socket
> + * @optval: destination buffer
> + * @optlen: size of peer label copied into the buffer
> + * @len: maximum size of the destination buffer
> + *
> + * This hook allows the security module to provide peer socket security state
> + * for unix or connected tcp sockets to userspace via getsockopt
> + * SO_GETPEERCONTEXT.  For tcp sockets this can be meaningful if the socket
> + * is associated with an ipsec SA.
> + *
> + * Return: Returns 0 if all is well, otherwise, typical getsockopt return
> + *         values.
> + */
> +int security_socket_getpeerctx_stream(struct socket *sock, sockptr_t optval,
> +				      sockptr_t optlen, unsigned int len)
> +{
> +	struct security_hook_list *hp;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeerctx_stream,
> +			     list)
> +		return hp->hook.socket_getpeerctx_stream(sock, optval, optlen,
> +							 len);
> +
> +	return LSM_RET_DEFAULT(socket_getpeerctx_stream);
> +}

Don't we need the same magic that we have in security_getselfattr() to
handle the multi-LSM case?

--
paul-moore.com

Re: [PATCH RFC] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[Casey Schaufler]

On 6/20/2024 2:05 PM, Paul Moore wrote:
> On May 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> We recently introduced system calls to access process attributes that
>> are used by Linux Security Modules (LSM). An important aspect of these
>> system calls is that they provide the LSM attribute data in a format
>> that identifies the LSM to which the data applies. Another aspect is that
>> it can be used to provide multiple instances of the attribute for the
>> case where more than one LSM supplies the attribute.
>>
>> We wish to take advantage of this format for data about network peers.
>> The existing mechanism, SO_PEERSEC, provides peer security data as a
>> text string. This is sufficient when the LSM providing the information
>> is known by the user of SO_PEERSEC, and there is only one LSM providing
>> the information. It fails, however, if the user does not know which
>> LSM is providing the information.
>>
>> Discussions about extending SO_PEERSEC to accomodate either the new
> Spelling nitpick -> "accommodate" :)

Thanks.

>> format or some other encoding scheme invariably lead to the conclusion
>> that doing so would lead to tears. Hence, we introduce SO_PEERCONTEXT
>> which uses the same API data as the LSM system calls.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  arch/alpha/include/uapi/asm/socket.h  |  1 +
>>  arch/mips/include/uapi/asm/socket.h   |  1 +
>>  arch/parisc/include/uapi/asm/socket.h |  1 +
>>  arch/sparc/include/uapi/asm/socket.h  |  1 +
>>  include/linux/lsm_hook_defs.h         |  2 +
>>  include/linux/security.h              | 18 ++++++++
>>  include/uapi/asm-generic/socket.h     |  1 +
>>  net/core/sock.c                       |  4 ++
>>  security/apparmor/lsm.c               | 39 ++++++++++++++++
>>  security/security.c                   | 86 +++++++++++++++++++++++++++++++++++
>>  security/selinux/hooks.c              | 35 ++++++++++++++
>>  security/smack/smack_lsm.c            | 25 ++++++++++
>>  12 files changed, 214 insertions(+)
> ..
>
>> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
>> index 8ce8a39a1e5f..e0166ff53670 100644
>> --- a/include/uapi/asm-generic/socket.h
>> +++ b/include/uapi/asm-generic/socket.h
>> @@ -134,6 +134,7 @@
>>  
>>  #define SO_PASSPIDFD		76
>>  #define SO_PEERPIDFD		77
>> +#define SO_PEERCONTEXT		78
> Bikeshed time ... how about SO_PEERLSMCTX since we are returning a
> lsm_ctx struct?

Sure.


>> diff --git a/security/security.c b/security/security.c
>> index e387614cb054..fd4919c28e8f 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -874,6 +874,64 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
>>  	return rc;
>>  }
>>  
>> +/**
>> + * lsm_fill_socket_ctx - Fill a socket lsm_ctx structure
>> + * @optval: a socket LSM context to be filled
>> + * @optlen: uctx size
> "@optlen: @optval size"

Thank you.


>> + * @val: the new LSM context value
>> + * @val_len: the size of the new LSM context value
>> + * @id: LSM id
>> + * @flags: LSM defined flags
>> + *
>> + * Fill all of the fields in a lsm_ctx structure.  If @optval is NULL
>> + * simply calculate the required size to output via @optlen and return
>> + * success.
>> + *
>> + * Returns 0 on success, -E2BIG if userspace buffer is not large enough,
>> + * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated.
>> + */
>> +int lsm_fill_socket_ctx(sockptr_t optval, sockptr_t optlen, void *val,
>> +			size_t val_len, u64 id, u64 flags)
>> +{
>> +	struct lsm_ctx *nctx = NULL;
>> +	unsigned int nctx_len;
>> +	int loptlen;
> u32?

Probably. I'll revise in line with your comment below.

>> +	int rc = 0;
>> +
>> +	if (copy_from_sockptr(&loptlen, optlen, sizeof(int)))
>> +		return -EFAULT;
> It seems the current guidance prefers copy_safe_from_sockptr(), see
> the note in include/linux/sockptr.h.

Always a good idea to follow guidance.

>> +	nctx_len = ALIGN(struct_size(nctx, ctx, val_len), sizeof(void *));
>> +	if (nctx_len > loptlen && !sockptr_is_null(optval))
>> +		rc = -E2BIG;
> Why do we care if @optval is NULL or not?  We are in a -E2BIG state,
> we're not copying anything into @optval anyway.  In fact, why are we
> doing the @rc check below?  Do it here like we do in lsm_fill_user_ctx().
>
>   if (nctx_len > loptlen) {
>     rc = -E2BIG;
>     goto out;
>   }

That's a bit sloppy on my part. I'll clean it up.


>> +	/* no buffer - return success/0 and set @uctx_len to the req size */
> "... set @opt_len ... "

Yes.

>> +	if (sockptr_is_null(optval) || rc)
>> +		goto out;
> Do the @rc check above, not here.
>
>> +	nctx = kzalloc(nctx_len, GFP_KERNEL);
>> +	if (!nctx) {
>> +		rc = -ENOMEM;
>> +		goto out;
>> +	}
>> +	nctx->id = id;
>> +	nctx->flags = flags;
>> +	nctx->len = nctx_len;
>> +	nctx->ctx_len = val_len;
>> +	memcpy(nctx->ctx, val, val_len);
>> +
>> +	if (copy_to_sockptr(optval, nctx, nctx_len))
>> +		rc = -EFAULT;
> This is always going to copy to the start of @optval which means we
> are going to keep overwriting previous values in the multi-LSM case.

The multiple LSM case isn't handled in this version. I don't want this
patch to depend on multiple LSM support.

> I think we likely want copy_to_sockptr_offset(), or similar.  See my
> comment in security_socket_getpeerctx_stream().
>
>> +	kfree(nctx);
>> +out:
>> +	if (copy_to_sockptr(optlen, &nctx_len, sizeof(int)))
>> +		rc = -EFAULT;
>> +
>> +	return rc;
>> +}
>> +
>> +
>>  /*
>>   * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
>>   * can be accessed with:
>> @@ -4743,6 +4801,34 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
>>  	return LSM_RET_DEFAULT(socket_getpeersec_stream);
>>  }
>>  
>> +/**
>> + * security_socket_getpeerctx_stream() - Get the remote peer label
>> + * @sock: socket
>> + * @optval: destination buffer
>> + * @optlen: size of peer label copied into the buffer
>> + * @len: maximum size of the destination buffer
>> + *
>> + * This hook allows the security module to provide peer socket security state
>> + * for unix or connected tcp sockets to userspace via getsockopt
>> + * SO_GETPEERCONTEXT.  For tcp sockets this can be meaningful if the socket
>> + * is associated with an ipsec SA.
>> + *
>> + * Return: Returns 0 if all is well, otherwise, typical getsockopt return
>> + *         values.
>> + */
>> +int security_socket_getpeerctx_stream(struct socket *sock, sockptr_t optval,
>> +				      sockptr_t optlen, unsigned int len)
>> +{
>> +	struct security_hook_list *hp;
>> +
>> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeerctx_stream,
>> +			     list)
>> +		return hp->hook.socket_getpeerctx_stream(sock, optval, optlen,
>> +							 len);
>> +
>> +	return LSM_RET_DEFAULT(socket_getpeerctx_stream);
>> +}
> Don't we need the same magic that we have in security_getselfattr() to
> handle the multi-LSM case?

Yes. I would like to move this ahead independently of the multi-LSM support.
Putting the multi-LSM magic in is unnecessary and rather pointless until then.

> --
> paul-moore.com

Thank you for the review. Expect v2 before very long.

Re: [PATCH RFC] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[Paul Moore]

On Fri, Jun 21, 2024 at 12:06 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 6/20/2024 2:05 PM, Paul Moore wrote:
> > On May 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:

...

> >> +/**
> >> + * security_socket_getpeerctx_stream() - Get the remote peer label
> >> + * @sock: socket
> >> + * @optval: destination buffer
> >> + * @optlen: size of peer label copied into the buffer
> >> + * @len: maximum size of the destination buffer
> >> + *
> >> + * This hook allows the security module to provide peer socket security state
> >> + * for unix or connected tcp sockets to userspace via getsockopt
> >> + * SO_GETPEERCONTEXT.  For tcp sockets this can be meaningful if the socket
> >> + * is associated with an ipsec SA.
> >> + *
> >> + * Return: Returns 0 if all is well, otherwise, typical getsockopt return
> >> + *         values.
> >> + */
> >> +int security_socket_getpeerctx_stream(struct socket *sock, sockptr_t optval,
> >> +                                  sockptr_t optlen, unsigned int len)
> >> +{
> >> +    struct security_hook_list *hp;
> >> +
> >> +    hlist_for_each_entry(hp, &security_hook_heads.socket_getpeerctx_stream,
> >> +                         list)
> >> +            return hp->hook.socket_getpeerctx_stream(sock, optval, optlen,
> >> +                                                     len);
> >> +
> >> +    return LSM_RET_DEFAULT(socket_getpeerctx_stream);
> >> +}
> >
> > Don't we need the same magic that we have in security_getselfattr() to
> > handle the multi-LSM case?
>
> Yes. I would like to move this ahead independently of the multi-LSM support.
> Putting the multi-LSM magic in is unnecessary and rather pointless until then.

Starting with the LSM syscalls, I want any new user visible API that
can support multiple LSMs to have support for multiple LSMs.  Yes, the
setselfattr API doesn't support multiple LSMs, but that is because we
agreed there was never going to be a way to safely support that usage.
In this particular case, that same argument does not apply, we could
have multiple LSMs returning a socket's network peer information (even
if we don't currently see that), so let's make sure our API supports
it from the start.

Unrelated to the above, it would also be good to datagram support as a
patch 2/2 thing in a future version of this patchset.  Please be
careful not to carry over the mistakes we made with SCM_SECURITY (see
the GH discussion linked below).

* https://github.com/SELinuxProject/selinux-kernel/issues/24

-- 
paul-moore.com

Re: [PATCH RFC] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[Casey Schaufler]

On 6/21/2024 12:41 PM, Paul Moore wrote:
> On Fri, Jun 21, 2024 at 12:06 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 6/20/2024 2:05 PM, Paul Moore wrote:
>>> On May 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
> ..
>
>>>> +/**
>>>> + * security_socket_getpeerctx_stream() - Get the remote peer label
>>>> + * @sock: socket
>>>> + * @optval: destination buffer
>>>> + * @optlen: size of peer label copied into the buffer
>>>> + * @len: maximum size of the destination buffer
>>>> + *
>>>> + * This hook allows the security module to provide peer socket security state
>>>> + * for unix or connected tcp sockets to userspace via getsockopt
>>>> + * SO_GETPEERCONTEXT.  For tcp sockets this can be meaningful if the socket
>>>> + * is associated with an ipsec SA.
>>>> + *
>>>> + * Return: Returns 0 if all is well, otherwise, typical getsockopt return
>>>> + *         values.
>>>> + */
>>>> +int security_socket_getpeerctx_stream(struct socket *sock, sockptr_t optval,
>>>> +                                  sockptr_t optlen, unsigned int len)
>>>> +{
>>>> +    struct security_hook_list *hp;
>>>> +
>>>> +    hlist_for_each_entry(hp, &security_hook_heads.socket_getpeerctx_stream,
>>>> +                         list)
>>>> +            return hp->hook.socket_getpeerctx_stream(sock, optval, optlen,
>>>> +                                                     len);
>>>> +
>>>> +    return LSM_RET_DEFAULT(socket_getpeerctx_stream);
>>>> +}
>>> Don't we need the same magic that we have in security_getselfattr() to
>>> handle the multi-LSM case?
>> Yes. I would like to move this ahead independently of the multi-LSM support.
>> Putting the multi-LSM magic in is unnecessary and rather pointless until then.
> Starting with the LSM syscalls, I want any new user visible API that
> can support multiple LSMs to have support for multiple LSMs.  Yes, the
> setselfattr API doesn't support multiple LSMs, but that is because we
> agreed there was never going to be a way to safely support that usage.
> In this particular case, that same argument does not apply, we could
> have multiple LSMs returning a socket's network peer information (even
> if we don't currently see that), so let's make sure our API supports
> it from the start.

OK. I'll put that in v2 as well.

>
> Unrelated to the above, it would also be good to datagram support as a
> patch 2/2 thing in a future version of this patchset.  Please be
> careful not to carry over the mistakes we made with SCM_SECURITY (see
> the GH discussion linked below).

That's "in my queue". I didn't want to spend time on it until I got
feedback on this one.

>
> * https://github.com/SELinuxProject/selinux-kernel/issues/24
>

Re: [PATCH RFC] LSM, net: Add SO_PEERCONTEXT for peer LSM data

[Paul Moore]

On Fri, Jun 21, 2024 at 6:00 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 6/21/2024 12:41 PM, Paul Moore wrote:
> > On Fri, Jun 21, 2024 at 12:06 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 6/20/2024 2:05 PM, Paul Moore wrote:
> >>> On May 13, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:

...

> > Unrelated to the above, it would also be good to datagram support as a
> > patch 2/2 thing in a future version of this patchset.  Please be
> > careful not to carry over the mistakes we made with SCM_SECURITY (see
> > the GH discussion linked below).
>
> That's "in my queue". I didn't want to spend time on it until I got
> feedback on this one.

Fair enough, thanks.

-- 
paul-moore.com