Message-ID: <CAEjxPJ4ObSaEG98jHhDtOssD1mF1fEAioJODa4bHKhZO=7KDGw@mail.gmail.com>
Date: Wed, 21 Aug 2024 09:37:47 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: Paul Moore <paul@...l-moore.com>
Cc: Ondrej Mosnacek <omosnace@...hat.com>, Jeongjun Park <aha310510@...il.com>, selinux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()

On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@...l-moore.com> wrote:
>
> On Tue, Aug 20, 2024 at 2:24 PM Stephen Smalley
> <stephen.smalley.work@...il.com> wrote:
> > On Mon, Aug 19, 2024 at 5:46 AM Ondrej Mosnacek <omosnace@...hat.com> wrote:
> > > I'm not sure if this is the solution we want to go with... Consider
> > > the following from af_smc(7):
> > >
> > > >   Usage modes
> > > >      Two usage modes are possible:
> > > >
> > > >      AF_SMC native usage
> > > >             uses the socket domain AF_SMC instead of AF_INET and AF_INET6.  Specify SMCPROTO_SMC for AF_INET compatible socket semantics, and SMC_PROTO_SMC6 for AF_INET6 respectively.
> > > >
> > > >      Usage of AF_INET socket applications with SMC preload library
> > > >             converts AF_INET and AF_INET6 sockets to AF_SMC sockets.  The SMC preload library is part of the SMC tools package.
> > > >
> > > >      SMC socket capabilities are negotiated at connection setup. If one peer is not SMC capable, further socket processing falls back to TCP usage automatically.
> > >
> > > This means that the SMC sockets are intended to be used (also) as a
> > > drop-in compatible replacement for normal TCP sockets in applications
> > > and they even fall back to TCP when the endpoints fail to negotiate
> > > communication via SMC. That's a situation similar to MPTCP, where we
> > > just mapped MPTCP sockets to the tcp_socket SELinux class, so that
> > > MPTCP can be swapped in place of TCP transparently without having to
> > > do extensive policy changes. We may want to consider the same/similar
> > > approach here.
> > >
> > > I briefly played with this idea a couple of months ago, when I was
> > > asked by someone at Red Hat about SMC sockets and their integration
> > > with SELinux. IIRC, when I tried to implement the MPTCP approach and
> > > adjusted the selinux-testsuite to test SMC similarly as TCP and MPTCP,
> > > I saw that the netlabel-related tests (may have been more, I don't
> > > remember) weren't passing out of the box like with MPTCP. However, the
> > > person then didn't follow up on my questions, so I didn't look into it
> > > further...
> > >
> > > I'm attaching the WIP patches I worked with, in case someone would
> > > like to continue the experiments.
> >
> > I am not in favor of your approach, for the following reasons:
> > 1. It would be backward-incompatible with any current code using
> > AF_SMC (although this could be addressed by making it conditional on a
> > new policy capability, so this is not too difficult to overcome),
> > 2. It would not allow any distinction to ever be made in policy
> > between SMC sockets and TCP sockets, so we could never allow one
> > without the other.
> >
> > Hence, I am still in favor of Jeongjun's patch to consistently treat
> > AF_SMC and (AF_INET, SOCK_STREAM, IPPROTO_SMC) sockets, and then if
> > someone wants to extend that support to also provide more complete
> > access controls and/or networking labeling, defer that to a future
> > patch.
>
> Without passing any judgement on the patches Ondrej submitted (I tend
> to ignore patches as attachments for various reasons), I do share
> Ondrej's concerns that this may not be as simple as suggested in the
> original patch in this thread.  I saw the same thing as Ondrej
> regarding the TCP fallback and that immediately raised a number of
> questions that I don't believe have been properly addressed yet.
>
> Someone needs to dig into how the standard SMC protocol works first to
> ensure we have the necessary access controls for the current code; my
> guess is that we are probably okay since the socket-level controls are
> fairly generic, but I'm not sure we've actually seen proper
> confirmation that everything is good from a conceptual standpoint.
> Once that is done, we need to examine how the TCP fallback works,
> specifically how are connections managed and are the existing TCP
> hooks sufficient for SMC (the early connection state stuff can be
> tricky) and how to distinguish between normal-TCP and SMC-TCP.
>
> Basically I'm looking for some basic design concepts and not simply a
> passing test without any understanding of why/how it passed.

At present, we are already applying the general socket layer access
controls to AF_SMC sockets; hence, existing policies can prevent or
allow use of AF_SMC sockets through that mechanism. This is useful for
reducing kernel attack surface, e.g. prevent all use of AF_SMC by
untrusted code, or to limit use of AF_SMC to specific
processes/programs.

Since kernel commit d25a92ccae6bed02327b63d138e12e7806830f78
("net/smc: Introduce IPPROTO_SMC"), there is a way to bypass such
controls by creating such sockets via (AF_INET, SOCK_STREAM,
IPPROTO_SMC) instead of AF_SMC. In that situation, any process that is
allowed the socket layer permissions to the generic socket class would
be allowed to create/use SMC sockets.

Jeongjun's patch closes this bypass and ensures consistent application
of the general socket layer access controls for SMC sockets. Given
that, I don't see why we would defer merging it until someone figures
out a more complete solution for SMC sockets. It's more of a bug fix
than an enhancement.

Powered by blists - more mailing lists