Re: [PATCH v12 05/25] net: Prepare UDS for security module stacking

[Stephen Smalley <sds@xxxxxxxxxxxxx>]

On 12/16/19 5:36 PM, Casey Schaufler wrote:
> Change the data used in UDS SO_PEERSEC processing from a
> secid to a more general struct lsmblob. Update the
> security_socket_getpeersec_dgram() interface to use the
> lsmblob. There is a small amount of scaffolding code
> that will come out when the security_secid_to_secctx()
> code is brought in line with the lsmblob.
>
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Reviewed-by: John Johansen <john.johansen@xxxxxxxxxxxxx>
> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> cc: netdev@xxxxxxxxxxxxxxx

You list netdev in the body but it isn't on the cc list AFAICT.  It was 
copied on an earlier, possibly identical version of the series I 
believe.  Not sure what is required here and I know you've had problems 
with bounces.  Regardless, with respect to the code:

Acked-by: Stephen Smalley <sds@xxxxxxxxxxxxx>

> ---
>   include/linux/security.h |  7 +++++--
>   include/net/af_unix.h    |  2 +-
>   include/net/scm.h        |  8 +++++---
>   net/ipv4/ip_sockglue.c   |  8 +++++---
>   net/unix/af_unix.c       |  6 +++---
>   security/security.c      | 18 +++++++++++++++---
>   6 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 322ed9622819..995faba7393f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1356,7 +1356,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, char __user *optval,
>   				      int __user *optlen, unsigned len);
> -int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid);
> +int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> +				     struct lsmblob *blob);
>   int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>   void security_sk_free(struct sock *sk);
>   void security_sk_clone(const struct sock *sk, struct sock *newsk);
> @@ -1494,7 +1495,9 @@ static inline int security_socket_getpeersec_stream(struct socket *sock, char __
>   	return -ENOPROTOOPT;
>   }
>   
> -static inline int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
> +static inline int security_socket_getpeersec_dgram(struct socket *sock,
> +						   struct sk_buff *skb,
> +						   struct lsmblob *blob)
>   {
>   	return -ENOPROTOOPT;
>   }
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 3426d6dacc45..933492c08b8c 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -36,7 +36,7 @@ struct unix_skb_parms {
>   	kgid_t			gid;
>   	struct scm_fp_list	*fp;		/* Passed files		*/
>   #ifdef CONFIG_SECURITY_NETWORK
> -	u32			secid;		/* Security ID		*/
> +	struct lsmblob		lsmblob;	/* Security LSM data	*/
>   #endif
>   	u32			consumed;
>   } __randomize_layout;
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 1ce365f4c256..e2e71c4bf9d0 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -33,7 +33,7 @@ struct scm_cookie {
>   	struct scm_fp_list	*fp;		/* Passed files		*/
>   	struct scm_creds	creds;		/* Skb credentials	*/
>   #ifdef CONFIG_SECURITY_NETWORK
> -	u32			secid;		/* Passed security ID 	*/
> +	struct lsmblob		lsmblob;	/* Passed LSM data	*/
>   #endif
>   };
>   
> @@ -46,7 +46,7 @@ struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl);
>   #ifdef CONFIG_SECURITY_NETWORK
>   static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_cookie *scm)
>   {
> -	security_socket_getpeersec_dgram(sock, NULL, &scm->secid);
> +	security_socket_getpeersec_dgram(sock, NULL, &scm->lsmblob);
>   }
>   #else
>   static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_cookie *scm)
> @@ -97,7 +97,9 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
>   	int err;
>   
>   	if (test_bit(SOCK_PASSSEC, &sock->flags)) {
> -		err = security_secid_to_secctx(scm->secid, &secdata, &seclen);
> +		/* Scaffolding - it has to be element 0 for now */
> +		err = security_secid_to_secctx(scm->lsmblob.secid[0],
> +					       &secdata, &seclen);
>   
>   		if (!err) {
>   			put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index aa3fd61818c4..6cf57d5ac899 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -130,15 +130,17 @@ static void ip_cmsg_recv_checksum(struct msghdr *msg, struct sk_buff *skb,
>   
>   static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
>   {
> +	struct lsmblob lb;
>   	char *secdata;
> -	u32 seclen, secid;
> +	u32 seclen;
>   	int err;
>   
> -	err = security_socket_getpeersec_dgram(NULL, skb, &secid);
> +	err = security_socket_getpeersec_dgram(NULL, skb, &lb);
>   	if (err)
>   		return;
>   
> -	err = security_secid_to_secctx(secid, &secdata, &seclen);
> +	/* Scaffolding - it has to be element 0 */
> +	err = security_secid_to_secctx(lb.secid[0], &secdata, &seclen);
>   	if (err)
>   		return;
>   
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 7cfdce10de36..73d32f655f18 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -138,17 +138,17 @@ static struct hlist_head *unix_sockets_unbound(void *addr)
>   #ifdef CONFIG_SECURITY_NETWORK
>   static void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
>   {
> -	UNIXCB(skb).secid = scm->secid;
> +	UNIXCB(skb).lsmblob = scm->lsmblob;
>   }
>   
>   static inline void unix_set_secdata(struct scm_cookie *scm, struct sk_buff *skb)
>   {
> -	scm->secid = UNIXCB(skb).secid;
> +	scm->lsmblob = UNIXCB(skb).lsmblob;
>   }
>   
>   static inline bool unix_secdata_eq(struct scm_cookie *scm, struct sk_buff *skb)
>   {
> -	return (scm->secid == UNIXCB(skb).secid);
> +	return lsmblob_equal(&scm->lsmblob, &(UNIXCB(skb).lsmblob));
>   }
>   #else
>   static inline void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
> diff --git a/security/security.c b/security/security.c
> index cee032b5ce29..a3be3929a60a 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2108,10 +2108,22 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
>   				optval, optlen, len);
>   }
>   
> -int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
> +int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
> +				     struct lsmblob *blob)
>   {
> -	return call_int_hook(socket_getpeersec_dgram, -ENOPROTOOPT, sock,
> -			     skb, secid);
> +	struct security_hook_list *hp;
> +	int rc = -ENOPROTOOPT;
> +
> +	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_dgram,
> +			     list) {
> +		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> +			continue;
> +		rc = hp->hook.socket_getpeersec_dgram(sock, skb,
> +						&blob->secid[hp->lsmid->slot]);
> +		if (rc != 0)
> +			break;
> +	}
> +	return rc;
>   }
>   EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>