Re: [PATCH 08/14] Implement TSEM control plane.

[Greg KH <gregkh@linuxfoundation.org>]

On Sun, Feb 12, 2023 at 12:54:40AM -0600, Dr. Greg wrote:
> On Sat, Feb 11, 2023 at 11:59:19AM +0100, Greg KH wrote:
> 
> Looping in Paul Moore in order to get his thoughts.
> 
> > On Fri, Feb 10, 2023 at 06:18:06PM -0600, Dr. Greg wrote:
> > > On Thu, Feb 09, 2023 at 12:30:51PM +0100, Greg KH wrote:
> > > 
> > > Good afternoon Greg, thanks for taking the time to review the patches
> > > and pass along comments.
> > > 
> > > > On Fri, Feb 03, 2023 at 11:09:48PM -0600, Dr. Greg wrote:
> > > > > The fs.c file contains the implementation of the TSEM control
> > > > > plane that is in the form of a pseudo-filesystem mounted on the
> > > > > following directory:
> > > > > 
> > > > > /sys/fs/tsem
> > > 
> > > > Why are you using sysfs to mount this?
> 
> > > We followed the lead of the SMACK and SeLinux LSM's, both of which
> > > create the mount points for their control plane filesystems in
> > > /sys/fs.
> > > 
> > > In addition, as a filesystem, we chose to have tsemfs closely follow
> > > their design for continuity across the LSM's.  So they share similar
> > > functionality and design, modulo of course, the event description and
> > > trajectory export files that we will chat about below.
> > > 
> > > We can't use securityfs, secondary to the fact that it doesn't
> > > implement pollable files, which are a requirement for trust
> > > orchestrators based on external Trusted Modeling Agents.
> 
> > Why not fix securityfs to provide pollable files?  Other than that,
> > why can't you just use securityfs?
> 
> Now that we have had some additional bandwidth to look at issues after
> the first round release, it may be more straight forward to implement
> the pollable files in securityfs than we thought.  We will take
> another run at this and see what is possible without having to meddle
> with the internals of securityfs proper.

It's ok to mess around with securityfs to get it to work properly for
your use case, there's no reason to create a whole new filesystem just
because of one missing functionality.

> As the diffstat for the patch series indicates, we spent considerable
> time working to implement TSEM without touching anything outside its
> implementation directory.  I think this is something that anyone who
> has tried to upstream functionality into the mainline kernel would
> understand the merit of.

No, that's not how kernel development works, it's ok to touch other
portions when needed, otherwise you duplicate lots of extra code and
functionality as you are doing here.  Please do not do that.

> > You are creating a new structure-type-api here, why not use a
> > already-designed protocol instead like varlink if you need userspace
> > to parse events in an atomic way?  Or heck even json would be better
> > as there are universal userspace tools for that today.
> 
> As an industry, we are in the middle of a software supply chain
> security crisis.

That has nothing to do with the kernel, sorry.

> In a trust orchestrated architecture, the trust
> orchestrators, and their Sancho TMA implementations, are the most
> security critical components on the system.  Our objective is to keep
> the supply chain footprint for Quixote as small as possible.
> 
> To that point:
> 
> size /usr/local/lib/libyajl.so.2.1.1:
>    text    data     bss     dec     hex filename
>   33333     784      16   34133    8555 /usr/local/lib/libyajl.so.2.1.1
> 
> size /u/usr/sources/quixote-1.4/SecurityModel/EventParser.o
>    text    data     bss     dec     hex filename
>    2520       0       0    2520     9d8 /u/usr/sources/quixote-1.4/SecurityModel/EventParser.o
> 
> If we were to use JSON, we would use yajl, it is probably as light as
> anything out there.  Given that, on face value, this would represent
> over an order of magnitude increase in code size to achieve the same
> objective, plus add an external dependency.

So you require people to trust your custom parser and format just
because you don't want to rely on a trusted tool that the whole world
depends on?

Again, not a valid argument, sorry, please use common parsing tools
otherwise you are guaranteed to make mistakes and everyone will have to
rely on your custom tools only, which is not something that you would
accept from any other kernel change.

And I don't see a link to the userspace tools anywhere, did I miss it?

thanks,

greg k-h