On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Tue, Aug 20, 2024 at 2:24 PM Stephen Smalley
> <stephen.smalley.work@xxxxxxxxx> wrote:
> > On Mon, Aug 19, 2024 at 5:46 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> 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.
