Re: [PATCH v2 1/2] security, lsm: Introduce security_mptcp_add_subflow()

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Mon, 19 Dec 2022, Paolo Abeni wrote:

> MPTCP can create subflows in kernel context, and later indirectly
> expose them to user-space, via the owning mptcp socket.
>
> As discussed in the reported link, the above causes unexpected failures
> for server, MPTCP-enabled applications.
>
> Let's introduce a new LSM hook to allow the security module to relabel
> the subflow according to the owing process.
>
> Note that the new hook requires both the mptcp socket and the new
> subflow. This could allow future extensions, e.g. explicitly validating
> the mptcp <-> subflow linkage.
>
> Link: https://lore.kernel.org/mptcp/CAHC9VhTNh-YwiyTds=P1e3rixEDqbRTFj22bpya=+qJqfcaMfg@mail.gmail.com/
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
> - fix build issue with !CONFIG_SECURITY_NETWORK
> ---
> include/linux/lsm_hook_defs.h | 1 +
> include/linux/lsm_hooks.h     | 9 +++++++++
> include/linux/security.h      | 6 ++++++
> net/mptcp/subflow.c           | 6 ++++++
> security/security.c           | 5 +++++
> 5 files changed, 27 insertions(+)
>

Hi Paolo -

Thanks for the patches, the MPTCP content looks good to me:

Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index bd387d4b5a38..43b90784d914 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1680,6 +1680,10 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
>
> 	lock_sock(sf->sk);
>
> +	err = security_mptcp_add_subflow(sk, sf->sk);
> +	if (err)
> +		goto release_ssk;
> +
> 	/* the newly created socket has to be in the same cgroup as its parent */
> 	mptcp_attach_cgroup(sk, sf->sk);
>
> @@ -1692,6 +1696,8 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
> 	get_net_track(net, &sf->sk->ns_tracker, GFP_KERNEL);
> 	sock_inuse_add(net, 1);
> 	err = tcp_set_ulp(sf->sk, "mptcp");
> +
> +release_ssk:
> 	release_sock(sf->sk);
>
> 	if (err) {

--
Mat Martineau
Intel