[PATCH v3] selinux: do not report error on connect(AF_UNSPEC)

[PATCH v3] selinux: do not report error on connect(AF_UNSPEC)

[Paolo Abeni]

calling connect(AF_UNSPEC) on an already connected TCP socket is an
established way to disconnect() such socket. After commit 68741a8adab9
("selinux: Fix ltp test connect-syscall failure") it no longer works
and, in the above scenario connect() fails with EAFNOSUPPORT.

Fix the above explicitly early checking for AF_UNSPEC family, and
returning success in that case.

Suggested-by: Paul Moore <paul@paul-moore.com>
Fixes: 68741a8adab9 ("selinux: Fix ltp test connect-syscall failure")
Reported-by: Tom Deseyn <tdeseyn@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v2 -> v3:
 - do the check for AF_UNSPEC at the begining, as suggested by Paul
v1 -> v2:
 - avoid validation for AF_UNSPEC
---
 security/selinux/hooks.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c61787b15f27..3ec702cf46ca 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4637,6 +4637,14 @@ static int selinux_socket_connect_helper(struct socket *sock,
 	err = sock_has_perm(sk, SOCKET__CONNECT);
 	if (err)
 		return err;
+	if (addrlen < offsetofend(struct sockaddr, sa_family))
+		return -EINVAL;
+
+	/* connect(AF_UNSPEC) has special handling, as it is a documented
+	 * way to disconnect the socket
+	 */
+	if (address->sa_family == AF_UNSPEC)
+		return 0;
 
 	/*
 	 * If a TCP, DCCP or SCTP socket, check name_connect permission
@@ -4657,8 +4665,6 @@ static int selinux_socket_connect_helper(struct socket *sock,
 		 * need to check address->sa_family as it is possible to have
 		 * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
 		 */
-		if (addrlen < offsetofend(struct sockaddr, sa_family))
-			return -EINVAL;
 		switch (address->sa_family) {
 		case AF_INET:
 			addr4 = (struct sockaddr_in *)address;
-- 
2.20.1

Re: [PATCH v3] selinux: do not report error on connect(AF_UNSPEC)

[Paul Moore]

On Fri, May 10, 2019 at 1:13 PM Paolo Abeni <pabeni@redhat.com> wrote:
> calling connect(AF_UNSPEC) on an already connected TCP socket is an
> established way to disconnect() such socket. After commit 68741a8adab9
> ("selinux: Fix ltp test connect-syscall failure") it no longer works
> and, in the above scenario connect() fails with EAFNOSUPPORT.
>
> Fix the above explicitly early checking for AF_UNSPEC family, and
> returning success in that case.
>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Fixes: 68741a8adab9 ("selinux: Fix ltp test connect-syscall failure")
> Reported-by: Tom Deseyn <tdeseyn@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v2 -> v3:
>  - do the check for AF_UNSPEC at the begining, as suggested by Paul
> v1 -> v2:
>  - avoid validation for AF_UNSPEC
> ---
>  security/selinux/hooks.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Thanks Paolo, this looks good.  It sounded like DaveM wanted this to
go to -stable so I'll merge it and mark it as such; I think I will
wait until the end of this week just to see if there are any other
things which crop up during the merge window.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c61787b15f27..3ec702cf46ca 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4637,6 +4637,14 @@ static int selinux_socket_connect_helper(struct socket *sock,
>         err = sock_has_perm(sk, SOCKET__CONNECT);
>         if (err)
>                 return err;
> +       if (addrlen < offsetofend(struct sockaddr, sa_family))
> +               return -EINVAL;
> +
> +       /* connect(AF_UNSPEC) has special handling, as it is a documented
> +        * way to disconnect the socket
> +        */
> +       if (address->sa_family == AF_UNSPEC)
> +               return 0;
>
>         /*
>          * If a TCP, DCCP or SCTP socket, check name_connect permission
> @@ -4657,8 +4665,6 @@ static int selinux_socket_connect_helper(struct socket *sock,
>                  * need to check address->sa_family as it is possible to have
>                  * sk->sk_family = PF_INET6 with addr->sa_family = AF_INET.
>                  */
> -               if (addrlen < offsetofend(struct sockaddr, sa_family))
> -                       return -EINVAL;
>                 switch (address->sa_family) {
>                 case AF_INET:
>                         addr4 = (struct sockaddr_in *)address;
> --
> 2.20.1

-- 
paul moore
www.paul-moore.com

Re: [PATCH v3] selinux: do not report error on connect(AF_UNSPEC)

[Paul Moore]

On Mon, May 13, 2019 at 6:50 PM Paul Moore <paul@paul-moore.com> wrote:
> On Fri, May 10, 2019 at 1:13 PM Paolo Abeni <pabeni@redhat.com> wrote:
> > calling connect(AF_UNSPEC) on an already connected TCP socket is an
> > established way to disconnect() such socket. After commit 68741a8adab9
> > ("selinux: Fix ltp test connect-syscall failure") it no longer works
> > and, in the above scenario connect() fails with EAFNOSUPPORT.
> >
> > Fix the above explicitly early checking for AF_UNSPEC family, and
> > returning success in that case.
> >
> > Suggested-by: Paul Moore <paul@paul-moore.com>
> > Fixes: 68741a8adab9 ("selinux: Fix ltp test connect-syscall failure")
> > Reported-by: Tom Deseyn <tdeseyn@redhat.com>
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > v2 -> v3:
> >  - do the check for AF_UNSPEC at the begining, as suggested by Paul
> > v1 -> v2:
> >  - avoid validation for AF_UNSPEC
> > ---
> >  security/selinux/hooks.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
>
> Thanks Paolo, this looks good.  It sounded like DaveM wanted this to
> go to -stable so I'll merge it and mark it as such; I think I will
> wait until the end of this week just to see if there are any other
> things which crop up during the merge window.

Just a quick follow-up, I just merged this into selinux/stable-5.2 and
assuming the build/test runs clean overnight I'll send this to Linus
tomorrow.  Thanks again for the report and the fix.

-- 
paul moore
www.paul-moore.com