Re: [PATCH mptcp-net] mptcp: fix LSM labeling for passive msk

[Paul Moore <paul@paul-moore.com>]

On Wed, Dec 7, 2022 at 9:19 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
> On Wed, 7 Dec 2022, Paolo Abeni wrote:
>
> > MPTCP sockets created via accept() inherit their LSM label
> > from the initial request socket, which in turn get it from the
> > listener socket's first subflow. The latter is a kernel socket,
> > and get the relevant labeling at creation time.
> >
> > Due to all the above even the accepted MPTCP socket get a kernel
> > label, causing unexpected behaviour and failure on later LSM tests.
> >
> > Address the issue factoring out a socket creation helper that does
> > not include the post-creation LSM checks. Use such helper to create
> > mptcp subflow as in-kernel sockets and doing explicitly LSM validation:
> > vs the current user for the first subflow, as a kernel socket otherwise.
> >
> > Fixes: 0c14846032f2 ("mptcp: fix security context on server socket")
> > Reported-by: Ondrej Mosnacek <omosnace@redhat.com>
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
> The MPTCP content looks good to me:
>
> Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>
> I didn't see issues with the socket.c changes but I'd like to get some
> security community feedback before upstreaming - Paul or other security
> reviewers, what do you think?

Sorry, I was distracted by other things the past few days ...

One thing that jumps out is the potential for misuse of
__sock_create_nosec(); I can see people accidentally using this
function by accident in other areas of the stack and causing a new set
of problems.

We discussed this in the other thread, but there is an issue with
subflows being labeled based on the mptcp_subflow_create_socket()
caller and not the main MPTCP socket.

I know there is a desire to get a small (in size) patch to fix this,
but I think creating a new LSM hook may be the only way to solve this
in a sane manner.  My original thought was a new LSM hook call inside
mptcp_subflow_create_socket() right after the sock_create_kern() call.
The only gotcha is that it would occur after
security_socket_post_create(), but that should be easy enough to
handle inside the LSMs.

-- 
paul-moore.com