Re: [PATCH] integrity: Expose data structures required for include/linux/integrity.h

[Florent Revest]

On Wed, 2019-12-18 at 09:28 -0500, Mimi Zohar wrote:
> [Cc'ing Matthew]
> 
> On Wed, 2019-12-18 at 08:34 -0500, Mimi Zohar wrote:
> > On Wed, 2019-12-18 at 12:03 +0100, Florent Revest wrote:
> > > On Tue, 2019-12-17 at 18:08 -0500, Mimi Zohar wrote:
> > > > On Tue, 2019-12-17 at 08:25 -0800, Casey Schaufler wrote:
> > > > > On 12/17/2019 5:47 AM, Florent Revest wrote:
> > > > > > From: Florent Revest <revest@xxxxxxxxxx>
> > > > > > 
> > > > > > include/linux/integrity.h exposes the prototype of
> > > > > > integrity_inode_get().
> > > > > > However, it relies on struct integrity_iint_cache which is
> > > > > > currently
> > > > > > defined in an internal header,
> > > > > > security/integrity/integrity.h.
> > > > > > 
> > > > > > To allow the rest of the kernel to use integrity_inode_get,
> > > > > 
> > > > > Why do you want to do this?
> > > > 
> > > > ditto
> > > 
> > > My team works on KRSI (eBPF MAC policies presented at LSS by KP
> > > Singh).
> > > https://lkml.org/lkml/2019/9/10/393 We identified file hashes
> > > gathered
> > > from the integrity subsystem as an interesting field that we
> > > could
> > > potentially someday expose to eBPF programs through helpers.
> > > 
> > > One of the reason behind writing KRSI is to replace a custom
> > > kernel
> > > auditing module that currently needs to redefine those structures
> > > to
> > > access them. I imagine other kernel modules could benefit from a
> > > file
> > > hash API too.
> > > 
> > > This is the least intrusive patch I could come up with that
> > > allows us
> > > to lookup a hash from an inode. I was surprised to find that
> > > integrity_inode_get was exposed but not the structures it
> > > returns.
> > > 
> > > If the community is interested in a different file hash API, I'd
> > > be
> > > happy to iterate on this patch based on your feedback.
> > 
> > There's a major difference between returning just the file hash and
> > making the integrity_iint_cache structure public. 

Certainly!
I am new to this subsystem so I just wanted to get the discussion
started. I am happy to make a more specific function.

> > Peter Moody's original code queried the cache[1].  Why do you need
> > access to the structure itself?
> > FYI, if/when we get to IMA namespacing, the cache structure will
> > change.
> > 
> > [1] ima: add the ability to query ima for the hash of a given file.
> 
> If you're using Peter's patch, or something similar, I'd appreciate
> your taking the time to upstream it.

Thank you for pointing me to Peter's patch! No one in my team was aware
of his work on this. Ugh!
It appears that Peter left the company while trying to upstream his
patch and the situation just got stuck there for 4+ years now.

If you are still positive about the idea of a ima_file_hash function, I
will take his v6 patch (this is the latest I could find on the
sourceforce archives of linux-ima-devel), rebase it, take your comments
into account and send a new version by the end of the week.

> Mimi
> 
> > > > > >  this patch
> > > > > > moves the definition of the necessary structures from a
> > > > > > private
> > > > > > header
> > > > > > to a global kernel header.