On 07/27, Matthieu Baerts wrote:
Hi Paul, Stanislav,
On 18/07/2023 18:14, Paul Moore wrote:
On Tue, Jul 18, 2023 at 11:21 AM Geliang Tang geliang.tang@suse.com wrote:
As is described in the "How to use MPTCP?" section in MPTCP wiki [1]:
"Your app can create sockets with IPPROTO_MPTCP as the proto: ( socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP); ). Legacy apps can be forced to create and use MPTCP sockets instead of TCP ones via the mptcpize command bundled with the mptcpd daemon."
But the mptcpize (LD_PRELOAD technique) command has some limitations [2]:
- it doesn't work if the application is not using libc (e.g. GoLang
apps)
- in some envs, it might not be easy to set env vars / change the way
apps are launched, e.g. on Android
- mptcpize needs to be launched with all apps that want MPTCP: we could
have more control from BPF to enable MPTCP only for some apps or all the ones of a netns or a cgroup, etc.
- it is not in BPF, we cannot talk about it at netdev conf.
So this patchset attempts to use BPF to implement functions similer to mptcpize.
The main idea is add a hook in sys_socket() to change the protocol id from IPPROTO_TCP (or 0) to IPPROTO_MPTCP.
[1] https://github.com/multipath-tcp/mptcp_net-next/wiki [2] https://github.com/multipath-tcp/mptcp_net-next/issues/79
v5:
- add bpf_mptcpify helper.
v4:
- use lsm_cgroup/socket_create
v3:
- patch 8: char cmd[128]; -> char cmd[256];
v2:
- Fix build selftests errors reported by CI
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/79 Signed-off-by: Geliang Tang geliang.tang@suse.com
include/linux/bpf.h | 1 + include/linux/lsm_hook_defs.h | 2 +- include/linux/security.h | 6 +- include/uapi/linux/bpf.h | 7 + kernel/bpf/bpf_lsm.c | 2 + net/mptcp/bpf.c | 20 +++ net/socket.c | 4 +- security/apparmor/lsm.c | 8 +- security/security.c | 2 +- security/selinux/hooks.c | 6 +- tools/include/uapi/linux/bpf.h | 7 + .../testing/selftests/bpf/prog_tests/mptcp.c | 128 ++++++++++++++++-- tools/testing/selftests/bpf/progs/mptcpify.c | 17 +++ 13 files changed, 187 insertions(+), 23 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/mptcpify.c
...
diff --git a/security/security.c b/security/security.c index b720424ca37d..bbebcddce420 100644 --- a/security/security.c +++ b/security/security.c @@ -4078,7 +4078,7 @@ EXPORT_SYMBOL(security_unix_may_send);
- Return: Returns 0 if permission is granted.
*/ -int security_socket_create(int family, int type, int protocol, int kern) +int security_socket_create(int *family, int *type, int *protocol, int kern) { return call_int_hook(socket_create, 0, family, type, protocol, kern); }
Using the LSM to change the protocol family is not something we want to allow. I'm sorry, but you will need to take a different approach.
@Paul: Thank you for your feedback. It makes sense and I understand.
@Stanislav: Despite the fact the implementation was smaller and reusing more code, it looks like we cannot go in the direction you suggested. Do you think what Geliang suggested before in his v3 [1] can be accepted?
(Note that the v3 is the same as the v1, only some fixes in the selftests.)
We have too many hooks in networking, so something that doesn't add a new one is preferable :-( Moreover, we already have a 'socket init' hook, but it runs a bit late.
Is existing cgroup/sock completely unworkable? Is it possible to expose some new bpf_upgrade_socket_to(IPPROTO_MPTCP) kfunc which would call some new net_proto_family->upgrade_to(IPPROTO_MPTCP) to do the surgery? Or is it too hacky?
Another option Alexei suggested is to add some fentry-like thing:
noinline int update_socket_protocol(int protocol) { return protocol; } /* TODO: ^^^ add the above to mod_ret set */
int __sys_socket(int family, int type, int protocol) { ...
protocol = update_socket_protocol(protocol);
... }
But it's also too problem specific it seems? And it's not cgroup-aware.