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

From: Paul Moore <paul@paul-moore.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: selinux@vger.kernel.org, Tom Deseyn <tdeseyn@redhat.com>,
	Richard Haines <richard_c_haines@btinternet.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Subject: Re: [PATCH v2] selinux: do not report error on connect(AF_UNSPEC)
Date: Fri, 10 May 2019 11:32:52 -0400	[thread overview]
Message-ID: <CAHC9VhRCdSPB8LuFTtBjLqrZAc3h8UsDJPJnYnMZmU3x_zWkTQ@mail.gmail.com> (raw)
In-Reply-To: <34870696b95f9cf48b5436df46e27dddd054858c.1557492319.git.pabeni@redhat.com>

On Fri, May 10, 2019 at 9:49 AM 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 skipping the checks when the address family is not
> AF_INET{4,6} - we don't have any port to validate, but leave the
> SCTP code path untouched, as it has specific constraints.
>
> Fixes: 68741a8adab9 ("selinux: Fix ltp test connect-syscall failure")
> Reported-by: Tom Deseyn <tdeseyn@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
>  - avoid validation for AF_UNSPEC
> ---
>  security/selinux/hooks.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

What was wrong with explicitly checking for AF_UNSPEC as I mentioned
in my last email?

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c61787b15f27..bccc4b3e6f57 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4674,12 +4674,13 @@ static int selinux_socket_connect_helper(struct socket *sock,
>                         break;
>                 default:
>                         /* Note that SCTP services expect -EINVAL, whereas
> -                        * others expect -EAFNOSUPPORT.
> +                        * others must handle this at the protocol level:
> +                        * connect(AF_UNSPEC) on a connected socket is
> +                        * a documented way disconnect the socket
>                          */
>                         if (sksec->sclass == SECCLASS_SCTP_SOCKET)
>                                 return -EINVAL;
> -                       else
> -                               return -EAFNOSUPPORT;
> +                       return 0;
>                 }
>
>                 err = sel_netport_sid(sk->sk_protocol, snum, &sid);
> --
> 2.20.1

-- 
paul moore
www.paul-moore.com