Message-ID: <CAHC9VhS3y4OCsMLMC9cALMAN1oz2VbTDJW67j1hzP7pjx4qv+w@mail.gmail.com>
Date: Fri, 30 Aug 2024 16:04:53 -0400
From: Paul Moore <paul@...l-moore.com>
To: Stephen Smalley <stephen.smalley.work@...il.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 Thu, Aug 29, 2024 at 9:51 AM Stephen Smalley
<stephen.smalley.work@...il.com> wrote:
>
> 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.

Possibly.  However, I don't feel like it is a very big ask to have
someone spend some time to provide a basic summary of the SMC protocol
and what might be needed from a SELinux perspective; that would help
increase my confidence in our SMC support significantly and perhaps
then I would feel comfortable with the simple mapping originally
proposed here.

-- 
paul-moore.com

Powered by blists - more mailing lists