netdev - Re: [RFC bpf-next v5] bpf: Force to MPTCP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 27 Jul 2023 19:36:06 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: Paul Moore <paul@...l-moore.com>, Geliang Tang <geliang.tang@...e.com>,
 Stanislav Fomichev <sdf@...gle.com>
Cc: Alexei Starovoitov <ast@...nel.org>, bpf@...r.kernel.org,
 netdev@...r.kernel.org, mptcp@...ts.linux.dev, apparmor@...ts.ubuntu.com,
 linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
 linux-kselftest@...r.kernel.org
Subject: Re: [RFC bpf-next v5] bpf: Force to MPTCP

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@...e.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@...e.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.)

Cheers,
Matt

[1] https://lore.kernel.org/r/cover.1688631200.git.geliang.tang@suse.com
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net