On Mon, 2020-02-10 at 10:04 +0200, Janne Karhunen wrote:

On Thu, Feb 6, 2020 at 4:14 PM Mimi Zohar [off-list ref] wrote:

quoted

The implications of exporting and removing records from the IMA-
measurement list needs to be considered carefully.  Verifying a TPM
quote will become dependent on knowing where the measurements are
stored.  The existing measurement list is stored in kernel memory and,
barring a kernel memory attack, is protected from modification.
 Before upstreaming this or a similar patch, there needs to be a
discussion as to how the measurement list will be protected once is it
exported to userspace.

This patch now attempts to address two very different scenarios.  The
first scenario is where userspace is requesting exporting and removing
of the measurement list records.  The other scenario is the kernel
exporting and removing of the measurement list records.  Conflating
these two different use cases might not be the right solution, as we
originally thought.

The kernel already exports the IMA measurement list to userspace via a
securityfs file.  From a userspace perspective, missing is the ability
of removing N number of records.  In this scenario, userspace would be
responsible for safely storing the measurements (e.g. blockchain).
 The kernel would only be responsible for limiting permission, perhaps
based on a capability, before removing records from the measurement
list.

This is a good point. I will adapt the patch to this.

quoted

In the kernel usecase, somehow the kernel would need to safely export
the measurement list, or some portion of the measurement list, to a
file and then delete that portion.  What protects the exported records
stored in a file from modification?

Are we looking at protecting this file from a root exploit and the
potential DOS it might cause? In the original patch the file was root
writable only. As far as further limitations go, the easiest would
probably be to use the file immutable bit. If the kernel opens the
file and sets the immutable bit, it is the only entity that can ever
write to it - not even another root task could directly write to it.
The kernel could, as long as it keeps the file open.

The problem being addressed is freeing kernel memory instead of
letting the measurement list grow unbounded.  One solution is to
remove measurement list records, as you did, but that changes the
existing userspace expectations of returning the entire measurement
list.  In the userspace scenario, removing measurement list records is
the requirement.  For the kernel scenario, I don't think it is a
requirement.

quoted

Instead of exporting the measurement records, one option as suggested
by Amir Goldstein, would be to use a vfs_tmpfile() to get an anonymous
file for backing store.  The existing securityfs measurement lists
would then read from this private copy of the anonymous file.

I've Cc'ed fsdevel for additional comments/suggestions.

I didn't quickly see what the actual problem is that the vfs_tmpfile
solves in this context, will check.

The existing IMA measurement list is by design, as coined by George
Wilson, a "deliberate memory leak".  Fixing the "Deliberate IMA event
log memory leak" should be the problem description.  Amir's suggestion
of using a vfs_tmpfile seems like a reasonable solution.

Mimi