On Wed, Dec 9, 2020 at 5:02 AM Paolo Abeni [off-list ref] wrote:

On Tue, 2020-12-08 at 18:35 -0500, Paul Moore wrote:

quoted

On Tue, Dec 8, 2020 at 10:35 AM Paolo Abeni [off-list ref] wrote:

quoted

Hello,

I'm sorry for the latency, I'll have limited internet access till
tomorrow.

On Fri, 2020-12-04 at 18:22 -0500, Paul Moore wrote:

quoted

For SELinux the issue is that we need to track state in the sock
struct, via sock->sk_security, and that state needs to be initialized
and set properly.

As far as I can see, for regular sockets, sk_security is allocated via:

- sk_prot_alloc() -> security_sk_alloc() for client/listener sockets
- sk_clone_lock() -> sock_copy() for server sockets

MPTCP uses the above helpers, sk_security should be initialized
properly.

At least for SELinux, the security_socket_post_create() hook is
critical too as that is where the SELinux sock/socket state values are
actually set; see selinux_socket_post_create() for the SELinux hook.

MPTCP sockets are created via the conventional sys_socket() call path
or sk_clone_lock(). MPTCP subflows are created via sock_create_kern()
or csk_af_ops->syn_recv_sock().

Overall the above matches what plain TCP does: client sockets and
listener sockets will hit selinux_socket_post_create(), server sockets
will hit security_sk_clone().

quoted

quoted

quoted

 Similarly with TCP request_sock structs, via
request_sock->{secid,peer_secid}.  Is the MPTCP code allocating and/or
otherwise creating socks or request_socks outside of the regular TCP
code?

Request sockets are easier, I guess/hope: MPTCP handles them very
closely to plain TCP.

Are there a calls to security_inet_conn_request() and
security_inet_csk_clone() in the MPTCP code path?  As an example look
at tcp_conn_request() and inet_csk_clone_lock() for IPv4.

MPTCP subflows call both the above, via the relevant TCP call-path.
MPTCP sockets calls security_inet_conn_request() for client sockets on
connect(), but it looks like we currently lack a call
to security_inet_csk_clone() for server MPTCP sockets, as they are
created via direct call to sk_clone_lock().

I think that could be easily handled with an MPTCP patch.

quoted

quoted

quoted

We would also be concerned about socket structs, but I'm
guessing that code reuses the TCP code based on what you've said.

Only the main MPTCP 'struct socket' is exposed to the user space, and
that is allocated via the usual __sys_socket() call-chain. I guess that
should be fine. If you could provide some more context (what I should
look after) I can dig more.

Hopefully the stuff above should help, if not let me know :)

yes, it helped, thanks!

My understanding is that the MPTCP implementation aligns with this
proposed patch - modulo the required changed mentioned above, which
looks like a MPTCP bug.

Great, thanks for taking the time to go through all this with me/us.
When you're ready with an updated patch(set), be sure to send it to
both the SELinux and LSM lists so we can look it over, ACK, etc.

Thanks!

-- 
paul moore
www.paul-moore.com