[RFC bpf-next v5] bpf: Force to MPTCP
Matthieu Baerts
matthieu.baerts at tessares.net
Fri Jul 28 15:15:03 UTC 2023
Hi Stanislav,
On 27/07/2023 20:01, Stanislav Fomichev wrote:
> 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 at 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 at 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 :-(
Thank you for your reply and the explanation, I understand.
> Moreover, we already have a 'socket init' hook, but it runs a bit late.
Indeed. And we cannot move it before the creation of the socket.
> 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?
I cannot judge if it is too hacky or not but if you think it would be
OK, please tell us :)
> 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.
It looks like it is what Geliang did in his v6. If it is the only
acceptable solution, I guess we can do without cgroup support. We can
continue the discussions in his v6 if that's easier.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
More information about the Linux-security-module-archive
mailing list