lkml.org 
[lkml]   [2021]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v5 15/16] ima: Move dentries into ima_namespace
    From
    Date
    On Fri, 2021-12-10 at 10:32 -0500, Stefan Berger wrote:
    > On 12/10/21 10:26, Mimi Zohar wrote:
    > > On Fri, 2021-12-10 at 09:26 -0500, James Bottomley wrote:
    > >> On Fri, 2021-12-10 at 09:17 -0500, Stefan Berger wrote:
    > >>> On 12/10/21 08:02, Mimi Zohar wrote:
    > >>>> On Fri, 2021-12-10 at 07:40 -0500, Stefan Berger wrote:
    > >>>>> On 12/10/21 07:09, Mimi Zohar wrote:
    > >>>>>> On Fri, 2021-12-10 at 12:49 +0100, Christian Brauner wrote:
    > >>>>>>>> There's still the problem that if you write the policy,
    > >>>>>>>> making the file disappear then unmount and remount
    > >>>>>>>> securityfs it will come back. My guess for fixing this is
    > >>>>>>>> that we only stash the policy file reference,
    > >>>>>>>> create it if NULL but then set the pointer to PTR_ERR(-
    > >>>>>>>> EINVAL) or something and refuse to create it for that
    > >>>>>>>> value.
    > >>>>>>> Some sort of indicator that gets stashed in struct ima_ns
    > >>>>>>> that the file does not get recreated on consecutive mounts.
    > >>>>>>> That shouldn't be hard to fix.
    > >>>>>> The policy file disappearing is for backwards compatibility,
    > >>>>>> prior to being able to extend the custom policy. For embedded
    > >>>>>> usecases, allowing the policy to be written exactly once might
    > >>>>>> makes sense. Do we really want/need to continue to support
    > >>>>>> removing the policy in namespaces?
    > >>>>> I don't have an answer but should the behavior for the same
    > >>>>> #define in this case be different for host and namespaces? Or
    > >>>>> should we just 'select IMA_WRITE_POLICY and IMA_READ_POLICY' when
    > >>>>> IMA_NS is selected?
    > >>>> The latter option sounds good. Being able to analyze the namespace
    > >>>> policy is really important.
    > >>> Ok, I will adjust the Kconfig for this then. This then warrants the
    > >>> question whether to move the dentry into the ima_namespace. The
    > >>> current code looks like this.
    > >>>
    > >>> #if !defined(CONFIG_IMA_WRITE_POLICY) &&
    > >>> !defined(CONFIG_IMA_READ_POLICY)
    > >>> securityfs_remove(ns->policy_dentry);
    > >>> ns->policy_dentry = NULL;
    > >>> ns->policy_dentry_removed = true;
    > >>> #elif defined(CONFIG_IMA_WRITE_POLICY)
    > >>>
    > >>> With IMA_NS selecting IMA_WRITE_POLICY and IMA_READ_POLICY the above
    > >>> wouldn't be necessary anymore but I find it 'cleaner' to still have
    > >>> the dentry isolated rather than it being a global static as it was
    > >>> before...
    > >> This is really, really why you don't want the semantics inside the
    > >> namespace to differ from those outside, because it creates confusion
    > >> for the people reading the code, especially with magically forced
    > >> config options like this. I'd strongly suggest you either keep the
    > >> semantic in the namespace or eliminate it entirely.
    > >>
    > >> If you really, really have to make the namespace behave differently,
    > >> then use global variables and put a big comment on that code saying it
    > >> can never be reached once CONFIG_IMA_NS is enabled.
    > > The problem seems to be with removing the securityfs policy file.
    > > Instead of removing it, just make it inacessible for the "if
    > > !defined(CONFIG_IMA_WRITE_POLICY) && !defined(CONFIG_IMA_READ_POLICY)"
    > > case.
    >
    > So we would then leave it up to the one building the kernel to select
    > the proper compile time options (suggested ones being IMA_WRITE_POLICY
    > and IMA_READ_POLICY being enabled?) and behavior of host and IMA
    > namespace is then the same per those options? Removing the file didn't
    > seem the problem to me but more like whether the host should ever behave
    > differently from the namespace.

    You proposed "select IMA_WRITE_POLICY and IMA_READ_POLICY'" when IMA_NS
    is selected. At least IMA_READ_POLICY should be enabled for
    namespaces.

    In addition, if removing the securityfs file after a custom policy is
    loaded complicates namespacing, then don't remove it.

    thanks,

    Mimi

    \
     
     \ /
      Last update: 2021-12-10 16:50    [W:4.767 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site