| Message ID | 20230419-upstream-lsm-next-20230419-mptcp-sublows-user-ctx-v1-1-9d4064cb0075@tessares.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | security: SELinux/LSM label with MPTCP and accept | expand |
On Wed, Apr 19, 2023 at 1:44 PM Matthieu Baerts <matthieu.baerts@tessares.net> wrote: > > From: Paolo Abeni <pabeni@redhat.com> > > 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. "... according to the main MPTCP socket." You might also want to stick with a consistent capitalization of "MPTCP" in the commit description, but that is being *really* nitpicky on my part ;) There is a suggestion for some additional comments in the hook's description below, but otherwise this looks good to me. > 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> > Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net> > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> > --- > include/linux/lsm_hook_defs.h | 1 + > include/linux/security.h | 6 ++++++ > net/mptcp/subflow.c | 6 ++++++ > security/security.c | 15 +++++++++++++++ > 4 files changed, 28 insertions(+) ... > diff --git a/security/security.c b/security/security.c > index f4170efcddda..24cf2644a4b9 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -4667,6 +4667,21 @@ int security_sctp_assoc_established(struct sctp_association *asoc, > } > EXPORT_SYMBOL(security_sctp_assoc_established); > > +/** > + * security_mptcp_add_subflow() - Inherit the LSM label from the MPTCP socket > + * @sk: the owning MPTCP socket > + * @ssk: the new subflow > + * > + * Update the labeling for the given MPTCP subflow, to match the one of the > + * owning MPTCP socket. I would add a sentence at the end making it clear that this hook is called after the socket has been created and initialized via the security_socket_create() and security_socket_post_create() LSM hooks. > + * > + * Return: Returns 0 on success or a negative error code on failure. > + */ > +int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) > +{ > + return call_int_hook(mptcp_add_subflow, 0, sk, ssk); > +} > + > #endif /* CONFIG_SECURITY_NETWORK */ > > #ifdef CONFIG_SECURITY_INFINIBAND > > -- > 2.39.2 -- paul-moore.com
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 6bb55e61e8e8..7308a1a7599b 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -343,6 +343,7 @@ LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_association *asoc, struct sock *sk, struct sock *newsk) LSM_HOOK(int, 0, sctp_assoc_established, struct sctp_association *asoc, struct sk_buff *skb) +LSM_HOOK(int, 0, mptcp_add_subflow, struct sock *sk, struct sock *ssk) #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND diff --git a/include/linux/security.h b/include/linux/security.h index cd23221ce9e6..80a0b37a9f26 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1465,6 +1465,7 @@ void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk, struct sock *newsk); int security_sctp_assoc_established(struct sctp_association *asoc, struct sk_buff *skb); +int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk); #else /* CONFIG_SECURITY_NETWORK */ static inline int security_unix_stream_connect(struct sock *sock, @@ -1692,6 +1693,11 @@ static inline int security_sctp_assoc_established(struct sctp_association *asoc, { return 0; } + +static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) +{ + return 0; +} #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 4ae1a7304cf0..d361749cabff 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1692,6 +1692,10 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family, lock_sock_nested(sf->sk, SINGLE_DEPTH_NESTING); + 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); @@ -1704,6 +1708,8 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family, 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) { diff --git a/security/security.c b/security/security.c index f4170efcddda..24cf2644a4b9 100644 --- a/security/security.c +++ b/security/security.c @@ -4667,6 +4667,21 @@ int security_sctp_assoc_established(struct sctp_association *asoc, } EXPORT_SYMBOL(security_sctp_assoc_established); +/** + * security_mptcp_add_subflow() - Inherit the LSM label from the MPTCP socket + * @sk: the owning MPTCP socket + * @ssk: the new subflow + * + * Update the labeling for the given MPTCP subflow, to match the one of the + * owning MPTCP socket. + * + * Return: Returns 0 on success or a negative error code on failure. + */ +int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) +{ + return call_int_hook(mptcp_add_subflow, 0, sk, ssk); +} + #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND