On Mon, Nov 29, 2021 at 05:16:56PM +0100, Christian Brauner wrote: > On Mon, Nov 29, 2021 at 09:35:39AM -0600, Serge Hallyn wrote: > > On Mon, Nov 29, 2021 at 09:46:55AM -0500, James Bottomley wrote: > > > On Mon, 2021-11-29 at 15:22 +0100, Christian Brauner wrote: > > > > On Mon, Nov 29, 2021 at 09:10:29AM -0500, James Bottomley wrote: > > > > > On Mon, 2021-11-29 at 08:53 -0500, Stefan Berger wrote: > > > > > > On 11/29/21 07:50, James Bottomley wrote: > > > > > > > On Sun, 2021-11-28 at 22:58 -0600, Serge E. Hallyn wrote: > > > > > > > > On Sat, Nov 27, 2021 at 04:45:49PM +0000, James Bottomley > > > > > > > > wrote: > > > > > > > > > Currently we get one entry in the IMA log per unique file > > > > > > > > > event. So, if you have a measurement policy and it > > > > > > > > > measures a particular binary it will not get measured again > > > > > > > > > if it is subsequently executed. For Namespaced IMA, the > > > > > > > > > correct behaviour seems to be to log once per inode per > > > > > > > > > namespace (so every unique execution in a namespace gets a > > > > > > > > > separate log entry). Since logging once per inode per > > > > > > > > > namespace is > > > > > > > > I suspect I'll need to do a more in depth reading of the > > > > > > > > existing code, but I'll ask the lazy question anyway (since > > > > > > > > you say "the correct behavior seems to be") - is it actually > > > > > > > > important that files which were appraised under a parent > > > > > > > > namespace's policy already should be logged again? > > > > > > > I think so. For a couple of reasons, assuming the namespace > > > > > > > eventually gets its own log entries, which the next incremental > > > > > > > patch proposed to do by virtualizing the securityfs > > > > > > > entries. If you don't do this: > > > > > > > > > > > > To avoid duplicate efforts, an implementation of a virtualized > > > > > > securityfs is in this series here: > > > > > > > > > > > > https://github.com/stefanberger/linux-ima-namespaces/commits/v5.15%2Bimans.20211119.v3 > > > > > > > > > > > > It starts with 'securityfs: Prefix global variables with > > > > > > secruityfs_' > > > > > > > > > > That's quite a big patch series. I already actually implemented > > > > > this as part of the RFC for getting the per namespace measurement > > > > > log. The attached is basically what I did. > > > > > > > > > > Most of the time we don't require namespacing the actual virtualfs > > > > > file, because it's world readable. IMA has a special requirement > > > > > in this regard because the IMA files should be readable (and > > > > > writeable when we get around to policy updates) by the admin of the > > > > > namespace but their protection is 0640 or 0440. I thought the > > > > > simplest solution would be to make an additional flag that coped > > > > > with the permissions and a per-inode flag way of making the file as > > > > > "accessible by userns admin". Doing something simple like this > > > > > gives a much smaller diffstat: > > > > > > > > That's a NAK from me. Stefan's series might be bigger but it does > > > > things correctly. I appreciate the keep it simple attitude but no. I > > > > won't speciale-case securityfs or similar stuff in core vfs helpers. > > > > > > Well, there's a reason it's an unpublished patch. However, the more > > > important point is that namespacing IMA requires discussion of certain > > > points that we never seem to drive to a conclusion. Using the akpm > > > method, I propose simple patches that drive the discussion. I think > > > the points are: > > > > > > 1. Should IMA be its own namespace or tied to the user namespace? The > > > previous patches all took the separate Namespace approach, but I > > > think that should be reconsidered now keyrings are in the user > > > namespace. > > > > Well that purely depends on the needed scope. > > > > The audit container identifier is a neat thing. But it absolutely must > > be settable, so seems to conflict with your needs. > > > > Your patch puts an identifier on the user_namespace. I'm not quite sure, > > does that satisfy Stefan's needs? A new ima ns if and only if there is a > > new user ns? > > I kept thinking about this question while I was out running and while I > admittedly have reacted poorly to CLONE_NEWIMA patches before it feels > to me that this is the right approach after all. Making it part of > userns at least in this form isn't clean. > > I think attaching a uuid to a userns alone for the sake of IMA is wrong. > Additionally, I think a uuid only for the userns is too limited. This is > similar to the problem of the audit container id. If we have some sort > of uuid for ima it will automatically evolve into something like a > container id (I'm not even arguing that this is necessarily wrong.). > We also have the issue that we then have the container audit id thing - > if this ever lands and the ima userns uuid. All that makes it quite > messy. > > I think CLONE_NEWIMA is ultimately nicer and allows the implementation > to be decoupled from the userns and self-contained as possible. This > also means that ima ns works for privileged containers which sure is a > valid use-case. > It will also make securityfs namespacing easier as it can be a keyed > super where the key is the ima ns (similar to how we deal with e.g. > mqueue). s/ima ns/userns/g which is what Stefan already did in the link he shared.
