Re: [PATCH v7 00/14] ima: Namespace IMA with audit support in IMA-ns

[Christian Brauner]

On Fri, Dec 17, 2021 at 09:38:16PM -0500, Stefan Berger wrote:
> 
> On 12/16/21 08:31, Christian Brauner wrote:
> > 
> > 1. namespace securityfs
> >     This patch is thematically standalone and should move to the
> >     beginning of the series.
> >     I would strongly recommend to fold patch 9 and 10 into a single patch
> >     and add a lengthy explanation. You should be able to recycle a lof of
> >     stuff I wrote in earlier reviews.
> > 
> > 2. Introduce struct ima_namespace and pass it through to all callers:
> >     - introduce struct ima_namespace
> >     - move all the relevant things into this structure (this also avoids
> >       the "avoid_zero_size" hack).
> 
> 
> We could defer the kmalloc() that doesn't work on a zero-sized request. I
> would say this  is minor.
> 
> 
> >     - define, setup, and expose init_ima_ns
> >     - introduce get_current_ns() and always have it return &init_ima_ns for now
> >     - replace all accesses to global variables to go through &init_ima_ns
> >     - add new infrastructure you'll need later on
> >     Bonus is that you can extend all the functions that later need access
> >     to a specific ima namespace to take a struct ima_namespace * argument
> >     and pass down &init_ima_ns down (retrieved via get_current_ns()). This
> >     will make the actual namespace patch very easy to follow.
> > 
> > 3. namespace ima
> >     - add a new entry for struct ima_namespace to struct user_namespace
> >     - add creation helpers, kmem cache etc.
> >     - create files in securityfs per ns
> 
> I have tried this now and I am looking at 4 remaining patches that need to
> somehow find its way into v8 without causing too many disturbances. At what
> point (over how many patches) can I introduce CONFIG_IMA_NS without anything
> related to IMA namespacing happening? I need it early in 'your 3rd part'
> since it is also used for conditional compilation (Makefile) and #ifdef's
> where Makefile content and what the #ifdefs are doing probably shouldn't be
> squeezed into a single patch just so it's all enabled in one patch, but it
> should probably still remain logically separated into different patches.
> Enablement of IMA namespace would be in the very last patch. But there may
> be several patches between the very last one and CONFIG_IMA_NS is
> introduced...
> 
> v7 at least, before the requirement to do late/lazy initialization, enabled
> CONFIG_IMA_NS right away and built ever step on top of it, even if the IMA
> namespace only became **configurable** in the last patch when securityfs was
> enbled and one could set a policy. From that perspective it would be easier
> to switch to late initialization in a patch on top of v7 but .. ok, we
> cannot do that.
> 
> 
> > This way at all points in the series we have clearly defined semantics
> > where ima namespacing is either fully working or fully not working and
> > the switch is atomic in the patch(es) part of 3.
> Atomic over multiple patches? So introducing CONFIG_IMA_NS that doesn't do
> anything for several patches is still considered 'atomic' then ?

I was hoping I wouldn't need to do this but I think at this point it's
easier to illustrate what I mean than it is to try and explain it
sufficiently detailed.
Here's a __completely uncompiled and untested__ series illustrating the
organization I had in mind. This should hopefully answer all of your
questions. The series can be found at

git clone https://github.com/brauner/linux.git stefanberger.v5.15+imans.v8.v2

here's it for illustration (again, completely uncompiled and untested).
(It consists of all your patches but I've changed author and sign-off
for this excercise.):