Re: Broken SELinux/LSM labeling with MPTCP and accept(2)

[Paolo Abeni <pabeni@redhat.com>]

On Tue, 2022-12-06 at 15:43 +0100, Ondrej Mosnacek wrote:
> On Mon, Dec 5, 2022 at 9:58 PM Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > On Fri, 2022-12-02 at 15:16 -0500, Paul Moore wrote:
> [...]
> > > What if we added a new LSM call in mptcp_subflow_create_socket(), just
> > > after the sock_create_kern() call?
> > 
> > That should work, I think. I would like to propose a (last) attempt
> > that will not need an additional selinux hook - to try to minimize the
> > required changes and avoid unnecessary addional work for current and
> > future LSM mainteniance and creation.
> > 
> > I tested the following patch and passes the reproducer (and mptcp self-
> > tests). Basically it introduces and uses a sock_create_nosec variant,
> > to allow mptcp_subflow_create_socket() calling
> > security_socket_post_create() with the corrct arguments. WDYT?
> 
> This seems like a step in the right direction, but I wonder if we
> shouldn't solve the current overloading of the "kern" flag more
> explicitly - i.e. split it into two flags: one to indicate that the
> socket will only be used internally by the kernel ("internal") and
> another one to indicate if it should be labeled according to the
> current task or as a kernel-created socket ("kern"?). Technically,
> each combination could have a valid use case:
> - !internal && !kern -> a regular userspace-created socket,
> - !internal && kern -> a socket that is exposed to userspace, but
> created by the kernel outside of a syscall (e.g. some global socket
> created during initcall phase and later returned to userspace via an
> ioctl or something),
> - internal && !kern -> our MPTCP case, where the socket itself is
> internal, but the label is still important so it can be passed onto
> its accept-offspring (which may no longer be internal),
> - internal && kern -> a completely kernel-internal socket.

I would say perfect is the enemy of good ;) it would be nice to have a
fix sometime soon, and we can improve as needed.

> Another concern I have about this approach is whether it is possible
> (in some more advanced scenario) for mptcp_subflow_create_socket() to
> be called in the context of a different task than the one
> creating/handling the main socket. Because then a potential socket
> accepted from the new subflow socket would end up with an unexpected
> (and probably semantically wrong) label. Glancing over the call tree,
> it seems it can be called via some netlink commands - presumably
> intended to be used by mptcpd?

Yes, the above can happen, but I think it does not have LSM-related
implications, as subflows created in the above scenario can be MP_JOIN
only - that is, will never be even indirectly exposed to user-space.


Cheers,

Paolo