Re: [PATCH LSM v2 0/2] security: SELinux/LSM label with MPTCP and accept

[Ondrej Mosnacek]

On Thu, May 4, 2023 at 6:13 PM Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
> On Thu, 2023-05-04 at 16:14 +0200, Ondrej Mosnacek wrote:
> > On Thu, Apr 20, 2023 at 7:17 PM Matthieu Baerts
> > <matthieu.baerts@xxxxxxxxxxxx> wrote:
> > >
> > > In [1], Ondrej Mosnacek explained they discovered the (userspace-facing)
> > > sockets returned by accept(2) when using MPTCP always end up with the
> > > label representing the kernel (typically system_u:system_r:kernel_t:s0),
> > > while it would make more sense to inherit the context from the parent
> > > socket (the one that is passed to accept(2)). Thanks to the
> > > participation of Paul Moore in the discussions, modifications on MPTCP
> > > side have started and the result is available here.
> > >
> > > Paolo Abeni worked hard to refactor the initialisation of the first
> > > subflow of a listen socket. The first subflow allocation is no longer
> > > done at the initialisation of the socket but later, when the connection
> > > request is received or when requested by the userspace. This was a
> > > prerequisite to proper support of SELinux/LSM labels with MPTCP and
> > > accept. The last batch containing the commit ddb1a072f858 ("mptcp: move
> > > first subflow allocation at mpc access time") [2] has been recently
> > > accepted and applied in netdev/net-next repo [3].
> > >
> > > This series of 2 patches is based on top of the lsm/next branch. Despite
> > > the fact they depend on commits that are in netdev/net-next repo to
> > > support the new feature, they can be applied in lsm/next without
> > > creating conflicts with net-next or causing build issues. These two
> > > patches on top of lsm/next still passes all the MPTCP-specific tests.
> > > The only thing is that the new feature only works properly with the
> > > patches that are on netdev/net-next. The tests with the new labels have
> > > been done on top of them.
> > >
> > > Regarding the two patches, the first one introduces a new LSM hook
> > > called from MPTCP side when creating a new subflow socket. This hook
> > > allows the security module to relabel the subflow according to the owing
> > > process. The second one implements this new hook on the SELinux side.
> > >
> > > Link: https://lore.kernel.org/netdev/CAFqZXNs2LF-OoQBUiiSEyranJUXkPLcCfBkMkwFeM6qEwMKCTw@xxxxxxxxxxxxxx/ [1]
> > > Link: https://git.kernel.org/netdev/net-next/c/ddb1a072f858 [2]
> > > Link: https://lore.kernel.org/netdev/20230414-upstream-net-next-20230414-mptcp-refactor-first-subflow-init-v1-0-04d177057eb9@xxxxxxxxxxxx/ [3]
> > > Signed-off-by: Matthieu Baerts <matthieu.baerts@xxxxxxxxxxxx>
> > > ---
> > > Changes in v2:
> > > - Address Paul's comments, see the notes on each patch
> > > - Link to v1: https://lore.kernel.org/r/20230419-upstream-lsm-next-20230419-mptcp-sublows-user-ctx-v1-0-9d4064cb0075@xxxxxxxxxxxx
> > >
> > > ---
> > > Paolo Abeni (2):
> > >       security, lsm: Introduce security_mptcp_add_subflow()
> > >       selinux: Implement mptcp_add_subflow hook
> > >
> > >  include/linux/lsm_hook_defs.h |  1 +
> > >  include/linux/security.h      |  6 ++++++
> > >  net/mptcp/subflow.c           |  6 ++++++
> > >  security/security.c           | 17 +++++++++++++++++
> > >  security/selinux/hooks.c      | 16 ++++++++++++++++
> > >  security/selinux/netlabel.c   |  8 ++++++--
> > >  6 files changed, 52 insertions(+), 2 deletions(-)
> > > ---
> > > base-commit: d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> > > change-id: 20230419-upstream-lsm-next-20230419-mptcp-sublows-user-ctx-eee658fafcba
> > >
> > > Best regards,
> > > --
> > > Matthieu Baerts <matthieu.baerts@xxxxxxxxxxxx>
> > >
> >
> > I haven't yet looked closer at the code in this series, but I can at
> > least confirm that with the series (applied on top of net-next) the
> > selinux-testsuite now passes when run under mptcpize, with one caveat:
> >
> > The "client" test prog in the inet_socket subtest sets the SO_SNDTIMEO
> > socket option on the client socket, but the subtest takes
> > significantly longer to complete than when run without mptcpize. That
> > suggests to me that there is possibly some (pre-existing) issue with
> > MPTCP where the send/receive timeouts are not being passed to the
> > subflow socket(s), leading to a longer wait (I guess the default is
> > higher?)
>
> Indeed the behavior you describe is due to some mptcp bug in handling
> the SO_{SND,RCV}TIMEO socket tions, and it's really unrelated to the
> initially reported selinux issue.

Definitely unrelated, just wanted to report the bug :)

> If you could file an issue on our tracker, that would help ;)

I was about to ask where that tracker is, but then it occured to me to
check MAINTAINERS and the link is right there, so yes, will do :)


--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.