Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

On Thu, 2013-01-17 at 16:52 -0500, Vivek Goyal wrote:
> On Thu, Jan 17, 2013 at 11:46:57PM +0200, Kasatkin, Dmitry wrote:
> > On Thu, Jan 17, 2013 at 10:55 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > > On Thu, Jan 17, 2013 at 03:33:47PM -0500, Frank Ch. Eigler wrote:
> > >> Vivek Goyal <vgoyal@redhat.com> writes:
> > >>
> > >> > [...]
> > >> >> Can you please tell a bit more how this patch protect against direct
> > >> >> writing to the blocks?
> > >> >
> > >> > If you have loaded all the pages from disk and locked them in memory and
> > >> > verified the signature, then even if somebody modifies a block on disk
> > >> > it does not matter. We will not read pages from disk anymore for this
> > >> > exec(). We verified the signature of executable loaded in memory and
> > >> > in-memory copy is intact.
> > >>
> > >> Does this imply dramatically increasing physical RAM pressure and load
> > >> latency, because binaries (and presumably all their shared libraries)
> > >> have to be locked & loaded?  (Else if they are paged out to
> > >> encrypted-swap, is that sufficient protection against manipulation?)
> > >
> > > Even if you employ encrypted-swap, we still need to lock down any code
> > > and data which lives in executable file on disk to avoid the case of
> > > it being modified directly by writing to a block. Looks like IMA will not
> > > detect that case.
> > >
> > 
> > See my IMA patch I set today, which does locking the same way as you do.
> 
> Yes but I also mentioned that still there is little problem. Signature
> verification should happen after the pages have been locked and not
> before that.

My initial comments mentioned this.  We can either move the existing
ima_file_mmap() or add a new hook.

> Also I was thinking about encrypted swap. Any root process will have access
> to encrypted swap? If yes, then it atleast does not work for the use case
> I am trying to solve.

Dmitry's patch example does exactly what you did, setting MAP_LOCKED
before the mmap, but does it for all ELF executables.  This could be
configurable.  I would suggest looking at the IMA policy.

> By selectively signing root executable, I am differentiating it with rest
> of the root executable and not trusting root process here till it is
> signed. So if another root process can get to swap and modify its contents
> and it modified the address space of signed process.

You're hard coding policy in the kernel and relying on userspace to only
sign specific files.

> So for the use case I am trying to solve, encrypted swap is not the
> solution. We have to lock down all of the process memory.

Like IMA-appraisal, your patches enforce integrity.  The LSM hooks were
originally defined for mandatory access control.  A parallel set of
hooks, called LIM, was proposed but were not upstreamed, as there
weren't any users other than IMA.  As a result, the IMA calls were
embedded directly into the vfs layer, except where the LSM and IMA hooks
were co-located.

James/Rusty please correct me if I'm wrong, but the new kernel module
signature verification should not be construed as a general license for
adding integrity verification in an ad-hoc manner, but was an exception
for the lack of a file descriptor.

thanks,

Mimi