[PATCH] selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()
Alexey Kodanev
alexey.kodanev at oracle.com
Thu May 10 09:28:57 UTC 2018
On 10.05.2018 01:02, Paul Moore wrote:
...
> I just had a better look at this and I believe that Alexey and Stephen
> are right: this is the best option. My apologies for the noise
> earlier. However, while looking at the code I think there are some
> additional necessary changes:
>
> * In the case of an SCTP socket, we should return -EINVAL, just as we
> do with other address families.
Right.
> * While not strictly related to AF_UNSPEC, we really should be passing
> the address family of the sockaddr, and not the socket, to functions
> that need to interpret the bind address/port.
That looks like a correct solution. I guess we need the same fix for
sctp_connectx(), in selinux_socket_connect_helper().
>
> I'm waiting for my kernel to compile so I haven't given this any
> sanity testing, but the patch below is what I think we need ...
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..5f30045b2053 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4576,6 +4576,7 @@ static int selinux_socket_post_create(struct socket *sock,
> int family,
> static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, i
> nt addrlen)
> {
> struct sock *sk = sock->sk;
> + struct sk_security_struct *sksec = sk->sk_security;
> u16 family;
> int err;
>
> @@ -4587,13 +4588,13 @@ static int selinux_socket_bind(struct socket *sock, stru
> ct sockaddr *address, in
> family = sk->sk_family;
> if (family == PF_INET || family == PF_INET6) {
> char *addrp;
> - struct sk_security_struct *sksec = sk->sk_security;
> struct common_audit_data ad;
> struct lsm_network_audit net = {0,};
> struct sockaddr_in *addr4 = NULL;
> struct sockaddr_in6 *addr6 = NULL;
> unsigned short snum;
> u32 sid, node_perm;
> + u16 family_sa = address->sa_family;
>
> /*
> * sctp_bindx(3) calls via selinux_sctp_bind_connect()
> @@ -4601,11 +4602,19 @@ static int selinux_socket_bind(struct socket *sock, stru
> ct sockaddr *address, in
> * need to check address->sa_family as it is possible to have
> * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
> */
> - switch (address->sa_family) {
> + switch (family_sa) {
> + case AF_UNSPEC:
> case AF_INET:
> if (addrlen < sizeof(struct sockaddr_in))
> return -EINVAL;
> addr4 = (struct sockaddr_in *)address;
> + if (family_sa == AF_UNSPEC) {
> + /* see "__inet_bind()", we only want to allow
> + * AF_UNSPEC if the address is INADDR_ANY */
> + if (addr4->sin_addr.s_addr != htonl(INADDR_ANY))
> + goto err_af;
> + family_sa = AF_INET;
> + }
> snum = ntohs(addr4->sin_port);
> addrp = (char *)&addr4->sin_addr.s_addr;
> break;
> @@ -4617,15 +4626,14 @@ static int selinux_socket_bind(struct socket *sock, stru
> ct sockaddr *address, in
> addrp = (char *)&addr6->sin6_addr.s6_addr;
> break;
> default:
> - /* Note that SCTP services expect -EINVAL, whereas
> - * others expect -EAFNOSUPPORT.
> - */
> - if (sksec->sclass == SECCLASS_SCTP_SOCKET)
> - return -EINVAL;
> - else
> - return -EAFNOSUPPORT;
> + goto err_af;
> }
>
> + ad.type = LSM_AUDIT_DATA_NET;
> + ad.u.net = &net;
> + ad.u.net->sport = htons(snum);
> + ad.u.net->family = family_sa;
> +
May be we could move setting ad.u.net->v{4|6}info.saddr here as well?
Will send a v2 of this patch so that SCTP socket returns EINVAL with
AF_UNSPEC. Should I prepare a patch with correcting 'ad.u.net->family'
and sel_netnode_sid()?
Thanks,
Alexey
> if (snum) {
> int low, high;
>
> @@ -4637,10 +4645,6 @@ static int selinux_socket_bind(struct socket *sock, struc
> t sockaddr *address, in
> snum, &sid);
> if (err)
> goto out;
> - ad.type = LSM_AUDIT_DATA_NET;
> - ad.u.net = &net;
> - ad.u.net->sport = htons(snum);
> - ad.u.net->family = family;
> err = avc_has_perm(&selinux_state,
> sksec->sid, sid,
> sksec->sclass,
> @@ -4672,16 +4676,11 @@ static int selinux_socket_bind(struct socket *sock, stru
> ct sockaddr *address, in
> break;
> }
>
> - err = sel_netnode_sid(addrp, family, &sid);
> + err = sel_netnode_sid(addrp, family_sa, &sid);
> if (err)
> goto out;
>
> - ad.type = LSM_AUDIT_DATA_NET;
> - ad.u.net = &net;
> - ad.u.net->sport = htons(snum);
> - ad.u.net->family = family;
> -
> - if (address->sa_family == AF_INET)
> + if (family_sa == AF_INET)
> ad.u.net->v4info.saddr = addr4->sin_addr.s_addr;
> else
> ad.u.net->v6info.saddr = addr6->sin6_addr;
> @@ -4694,6 +4693,12 @@ static int selinux_socket_bind(struct socket *sock, struc
> t sockaddr *address, in
> }
> out:
> return err;
> +err_af:
> + /* Note that SCTP services expect -EINVAL, others -EAFNOSUPPORT. */
> + if (sksec->sclass == SECCLASS_SCTP_SOCKET)
> + return -EINVAL;
> + else
> + return -EAFNOSUPPORT;
> }
>
> /* This supports connect(2) and SCTP connect services such as sctp_connectx(3)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the Linux-security-module-archive
mailing list