lkml.org 
[lkml]   [2008]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/6] integrity: IMA as an integrity service provider
From
Date
On Wed, 2008-12-03 at 13:24 -0500, Mimi Zohar wrote:
> On Wed, 2008-12-03 at 08:03 -0500, Christoph Hellwig wrote:
> > On Tue, Dec 02, 2008 at 03:35:25PM -0800, Dave Hansen wrote:
> > > I have memories of talking about this bit. I was confused and you
> > > explained it to me, but it still isn't explained in the code. :( Again,
> > > I'm not convinced that this works. Can the code convince me, or should
> > > I go digging in my inbox?
> >
> > I also haven't seen a good explanation for it yet.
>
> Previous posting:
> "The OS protects against an executable file, already open for write,
> from being executed; and an executable file, open for execute, from
> being modified. In the same vein, we want to know that the file
> measured is the same file read, that it hasn't been modified. So, if a
> file already open for read is then opened for write, we log it with a
> "Time of Measure, Time of Use" error (ToMToU) and invalidate the
> PCR.....
>
> This is important when measuring configuration files, shell scripts (not
> measured in brpm_check_integrity which are protected by the OS), and
> files imported/included by scripts."
>
> Another posting:
> "From an integrity perspective, a file measurement might be invalidated
> unnecessarily, but it is safe. For any file when opened for write, while
> having an existing reader, will cause the file measurement to be
> invalidated."

Those are all great explanations. But, some of that needs to get in the
patch somehow. This is a subtle thing and someone looking at this a
year for now is going to have difficulty understanding why it was done.

> I'm just not seeing a problem. Perhaps because only regular files are
> being measured, and of them, only those defined by the policy, which
> most likely would not include pseudo filesystems (i.e. sysfs, procfs,
> tmpfs, securityfs).

There is no practical problem if you have false-positives on this check
and do extra invalidations. But, I think both Christoph and I are
nervous that this check is racy and there may be false-negatives and
thus may miss some invalidations (which would be harmful).

The check is racy which is cause for concern by itself. But, with
careful consideration, it may not be a dangerous or harmful race. Could
you please consider it carefully and share some of your thoughts in a
comment in the next version?

You need to check very, very carefully that there are no possible ways
for i_writecount to be elevated without a corresponding elevation of
d_count. I'm especially concerned as I look at some of the mmap() code.
It appears that there are some temporary i_writecount elevations as
VM_DENYWRITE is figured out. That needs some careful auditing to ensure
it doesn't violate what is being assumed in the integrity code.

-- Dave



\
 
 \ /
  Last update: 2008-12-03 19:53    [W:1.002 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site