2022-02-03 09:42:18

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v10 00/27] ima: Namespace IMA with audit support in IMA-ns

On Tue, Feb 01, 2022 at 03:37:08PM -0500, Stefan Berger wrote:
> The goal of this series of patches is to start with the namespacing of
> IMA and support auditing within an IMA namespace (IMA-ns) as the first
> step.
>
> In this series the IMA namespace is piggy backing on the user namespace
> and therefore an IMA namespace is created when a user namespace is
> created, although this is done late when SecurityFS is mounted inside
> a user namespace. The advantage of piggy backing on the user namespace
> is that the user namespace can provide the keys infrastructure that IMA
> appraisal support will need later on.
>
> We chose the goal of supporting auditing within an IMA namespace since it
> requires the least changes to IMA. Following this series, auditing within
> an IMA namespace can be activated by a user running the following lines
> that rely on a statically linked busybox to be installed on the host for
> execution within the minimal container environment:
>
> mkdir -p rootfs/{bin,mnt,proc}
> cp /sbin/busybox rootfs/bin
> cp /sbin/busybox rootfs/bin/busybox2
> echo >> rootfs/bin/busybox2
> PATH=/bin unshare --user --map-root-user --mount-proc --pid --fork \
> --root rootfs busybox sh -c \
> "busybox mount -t securityfs /mnt /mnt; \
> busybox echo 1 > /mnt/ima/active; \
> busybox echo 'audit func=BPRM_CHECK mask=MAY_EXEC' > /mnt/ima/policy; \
> busybox2 cat /mnt/ima/policy"
>
> [busybox2 is used to demonstrate 2 audit messages; see below]
>
> Following the audit log on the host the last line cat'ing the IMA policy
> inside the namespace would have been audited. Unfortunately the auditing
> line is not distinguishable from one stemming from actions on the host.
> The hope here is that Richard Brigg's container id support for auditing
> would help resolve the problem.
>
> In the above the writing of '1' to the 'active' file is used to activate
> the IMA namespace. Future extensions to IMA namespaces will make use of
> the configuration stage after the mounting of securityfs and before the
> activation to for example choose the measurement log template.
>
> The following lines added to a suitable IMA policy on the host would
> cause the execution of the commands inside the container (by uid 1000)
> to be measured and audited as well on the host, thus leading to two
> auditing messages for the 'busybox2 cat' above and log entries in IMA's
> system log.
>
> echo -e "measure func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
> "audit func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
> > /sys/kernel/security/ima/policy
>
> The goal of supporting measurement and auditing by the host, of actions
> occurring within IMA namespaces, is that users, particularly root,
> should not be able to evade the host's IMA policy just by spawning
> new IMA namespaces, running programs there, and discarding the namespaces
> again. This is achieved through 'hierarchical processing' of file
> accesses that are evaluated against the policy of the namespace where
> the action occurred and against all namespaces' and their policies leading
> back to the root IMA namespace (init_ima_ns).
>
> The patch series adds support for a virtualized SecurityFS with a few
> new API calls that are used by IMA namespacing. Only the data relevant
> to the IMA namespace are shown. The files and directories of other
> security subsystems (TPM, evm, Tomoyo, safesetid) are not showing
> up when secruityfs is mounted inside a user namespace.
>
> Much of the code leading up to the virtualization of SecurityFS deals
> with moving IMA's variables from various files into the IMA namespace
> structure called 'ima_namespace'. When it comes to determining the
> current IMA namespace I took the approach to get the current IMA
> namespace (get_current_ns()) on the top level and pass the pointer all
> the way down to those functions that now need access to the ima_namespace
> to get to their variables. This later on comes in handy once hierarchical
> processing is implemented in this series where we walk the list of
> namespaces backwards and again need to pass the pointer into functions.
>
> This patch also introduces usage of CAP_MAC_ADMIN to allow access to the
> IMA policy via reduced capabilities. We would again later on use this
> capability to allow users to set file extended attributes for IMA
> appraisal support.
>
> My tree with these patches is here:
>
> git fetch https://github.com/stefanberger/linux-ima-namespaces v5.16+imans.v10.posted
>
> Regards,
> Stefan
>
> Links to previous postings:
> v1: https://lore.kernel.org/linux-integrity/[email protected]/T/#t
> v2: https://lore.kernel.org/linux-integrity/[email protected]/T/#t
> v3: https://lore.kernel.org/linux-integrity/[email protected]/T/#t
> v4: https://lore.kernel.org/linux-integrity/[email protected]/T/#t
> v5: https://lore.kernel.org/linux-integrity/[email protected]/T/#t
> v6: https://lore.kernel.org/linux-integrity/[email protected]/T/#t
> v7: https://lore.kernel.org/linux-integrity/20211217100659.2iah5prshavjk6v6@wittgenstein/T/#t
> v8: https://lore.kernel.org/all/[email protected]/#r
> v9: https://lore.kernel.org/linux-integrity/?t=20220131234353
>
> v10:
> - Added A-b's; addressed issues from v9
> - Added 2 patches to support freeing of iint after namespace deletion
> - Added patch to return error code from securityfs functions
> - Added patch to limit number of policy rules in IMA-ns to 1024

I'm going to go take a lighter touch with this round of reviews.
First, because I have February off. :)
Second, because I think that someone who is more familiar with IMA and
its requirements should take another look to provide input and ask more
questions. Last time I spoke to Serge he did want to give this a longer
look and maybe also has additional questions.


2022-02-03 20:37:36

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v10 00/27] ima: Namespace IMA with audit support in IMA-ns


On 2/2/22 09:13, Christian Brauner wrote:
> On Tue, Feb 01, 2022 at 03:37:08PM -0500, Stefan Berger wrote:
>>
>> v10:
>> - Added A-b's; addressed issues from v9
>> - Added 2 patches to support freeing of iint after namespace deletion
>> - Added patch to return error code from securityfs functions
>> - Added patch to limit number of policy rules in IMA-ns to 1024
> I'm going to go take a lighter touch with this round of reviews.
> First, because I have February off. :)
> Second, because I think that someone who is more familiar with IMA and
> its requirements should take another look to provide input and ask more
> questions. Last time I spoke to Serge he did want to give this a longer
> look and maybe also has additional questions.

The one problem I am seeing is that we probably cannot support auditing
in IMA namespaces since every user can now create an IMA namespace.
Unless auditing was namespaced, the way it is now gives too much control
to the user to flood the host audit log. So, we may need to head towards
support for IMA measurements in the IMA namespace right away and not
support audit rules but also possibly eliminate other actions that are
being audited by IMA to not occur while an IMA namespace is active, such
as when policy rules are being set etc. Not supporting auditing in
IMA-ns affects only few of the patches in this series. We need most of
them for a basis of IMA measurements but to get to IMA measurements
along with support for inheritance and configuration of hash algorithm
and log template etc. to use in the IMA namespace and set it in its
configuration 'stage' (before activation), we will need at least 25 more
patches on top of what have here now... so this series will then be
around 50 patches.

   Stefan