Re: [ANNOUNCE] Release Digsig 1.3.1: kernel module for run-time authentication of binaries

[Chris Wright <chrisw@osdl.org>]

* Makan Pourzandi (Makan.Pourzandi@ericsson.com) wrote:
> Hi all,
> 
> see below:
> 
> Chris Wright wrote:
> > * Makan Pourzandi (Makan.Pourzandi@ericsson.com) wrote:
> > 
> >>Serge E. Hallyn wrote:
> >>
> >>>Quoting Chris Wright (chrisw@osdl.org):
> >>>
> >>>>AFAICT, this means anybody with read access to a file can block all
> >>>>writes.  This doesn't sound great.
> >>>
> >>>True.
> >>
> >>>This could be fixed by adding a check at the top of dsi_file_mmap for
> >>>file->f_dentry->d_inode->i_mode & MAY_EXEC.  Of course then shared
> >>>libraries which are installed without execute permissions will cause
> >>>apps to break.  On my quick test, I couldn't run xterm or vi  :)
> >>>
> >>>Note that blocking writes requires that the file be a valid ELF file,
> >>>as this is all that digsig mediates.  So I'm not sure which we worry
> >>>about more - the fact that all shared libraries have to be installed
> >>>with execute permissions (under the proposed solution), or that write
> >>
> >>
> >>My 2 cents, a quick browsing on my machine (fedora core 1) shows that 
> >>almost all my shared libraries are installed with both execution and 
> >>read permissions.  IMHO, I don't believe then that this should be 
> >>considered as a major issue.
> > 
> > 
> > This has nothing to do with file permissions aside of read.  All you need
> > is read permission, then you can mmap(PROT_EXEC) which will kick off the
> > check, and do deny_write_access.  It's a freeform way to lock writers
> > out of any readable file in the system.  This is why MAP_EXECUTABLE and
> > MAP_DENYWRITE are masked off at syscall entry.
> 
> Chris, my understanding is that Serge's patch even though it restricts a 
> little the system admin, is the best solution we have for time being. Am 
> I right?
> If this is the case I'll include Serge's patch in the cvs source code 
> and send out a new release soon.

The key piece I had missed is that it only checks elf files (I now see
read_elf_header).  So, the test may even be superfluous.  But try it,
see what breaks, then send fixes upstream ;-)  BTW, the read_elf_header
returns NULL on error, but IS_ERR is used against the returned pointer
(so errors will oops).  Also, the security_set_operations +
set_digsig_ops looks like it should be replaced with static structure
member assignment.  The digsig_sem can be statically setup with
DECLARE_MUTEX().  digsig.c::secondary should be static.
digsig.c::digsig_num_cached_sigs should be static and declared in
digsig_cache.c.  I'll keep digging.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net