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. thanks, Mimi
