Message-ID: <CAEjxPJ4WkYo+6xbvrbz2XwmTYQeFEoYnWAjVtExnWvGiKZyvMQ@mail.gmail.com>
Date: Thu, 29 Aug 2024 09:50:59 -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 Wed, Aug 21, 2024 at 4:02 PM Paul Moore <paul@...l-moore.com> wrote:
>
> On Wed, Aug 21, 2024 at 9:38 AM Stephen Smalley
> <stephen.smalley.work@...il.com> wrote:
> > On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@...l-moore.com> wrote:
>
> ...
>
> > > 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.
>
> That's true.  I'm not suggesting we revert what we currently have, I'm
> only expressing some caution about moving forward with
> AF_INET/IPPROTO_SMC without a better understanding.  Ideally we would
> have done so before adding AF_SMC support, but we didn't, or at least
> I don't recall much discussion at the time.
>
> > 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.
>
> SCTP, that's why.  Granted, SCTP is likely a far more complicated
> protocol than SMC, but the TCP fallback raises all sorts of complexity
> red flags in my mind.  Before we go further with SMC I want to see
> some evidence that someone has looked through the SMC protocol and can
> write a few coherent paragraphs about how the SELinux access controls
> for the SMC protocol should work.
>
> ... and yes, labeled SCTP is still broken.  Perhaps someday soon I'll
> have the time to finish the patchset to fix it.

I see this as being different than SCTP. The AF_SMC support was
introduced with the extended_socket_class support which merely
introduced distinct socket security classes for each address family so
that SELinux can distinguish each address family in policy. Previously
a number of the socket address families were defaulting to the generic
socket security class and could not be distinguished in policy. The
only change was introducing a distinct security class specifically for
SMC sockets, not introducing any family/protocol-specific logic. Only
the socket layer access controls were being applied (both before and
after the introduction of the SMC socket class, just changing which
class was being used). Fixing the kernel to also map IPPROTO_SMC to
the SMC socket class likewise just ensure consistent application of
the socket layer access controls on SMC sockets regardless of how they
are created and doesn't introduce any SMC-specific logic.

Powered by blists - more mailing lists