All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Safford <safford@watson.ibm.com>
To: Kyle Moffett <kyle@moffetthome.net>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	James Morris <jmorris@namei.org>
Subject: Re: [PATCH v7 00/16] EVM
Date: Thu, 14 Jul 2011 11:07:46 -0400	[thread overview]
Message-ID: <1310656066.3845.244.camel@localhost> (raw)
In-Reply-To: <BANLkTims9aa3Ct+C1x6kNDeOi2juUNbzkA@mail.gmail.com>

On Thu, 2011-06-30 at 18:32 -0400, Kyle Moffett wrote:
> Whoops, resent in plain text, sorry about the HTML
> 
> On Wed, Jun 29, 2011 at 23:51, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2011-06-29 at 21:57 -0400, Kyle Moffett wrote:
> >> On Wed, Jun 29, 2011 at 19:42, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > On Wed, 2011-06-29 at 16:53 -0400, Kyle Moffett wrote:
> >> >> Hmm, I'm not sure that this design actually provides the protection that
> >> >> you claim it does.
> >> >>
> >> >> Specifically, you don't actually protect the on-disk data-structures that
> >> >> are far more vulnerable to malicious modification than the actual *values*
> >> >> of the extended attributes themselves.
> >> >
> >> > True, EVM only protects the file metadata. The patch description says,
> >> >
> >> >        While this patchset does authenticate the security xattrs, and
> >> >        cryptographically binds them to the inode, coming extensions
> >> >        will bind other directory and inode metadata for more complete
> >> >        protection.
> >> >
> >> > It should have said, "bind other directory, inode data and inode
> >> > metadata."
> >> >
> >> > In particular, IMA-appraisal stores the file data's hash as the
> >> > security.ima xattr, which is EVM protected. Other methods, such as
> >> > digital signatures, could be used instead of the file's hash, to
> >> > additionally provide authenticity.
> >>
> >> The problem is that your *design* assumes that the filesystem itself is
> >> valid, but your stated threat model assumes that the attacker has offline
> >> access to the filesystem, an explicit contradiction.
> >>
> >> There have been numerous cases in the past where a corrupt or invalid
> >> filesystem causes kernel panics or even exploitable overflows or memory
> >> corruption; see the history of the "fsfuzzer" tool for more information.
> >>
> >> Furthermore, if the attacker can intentionally cause data extent or inode
> >> extended attribute aliasing (shared space-on-disk) between different
> >> files then your entire security model falls flat.
> >>
> >> So if you assume the attacker has raw access to the underlying filesystem
> >> then you MUST authenticate *all* of the low-level filesystem data,
> >> including the "implicit" metadata of allocation tables, extents, etc.
> >>
> >> Cheers,
> >> Kyle Moffett
> >
> > Assuming someone does modify the underlying filesystem, how does that
> > break the security model?  The purpose of EVM/IMA-appraisal is not to
> > prevent files offline from being modified, but to detect if/when it
> > occurs and to enforce file integrity.
> 
> The problem is that you are assuming that a large chunk of filesystem
> code is capable of properly and securely handling untrusted and malicious
> content.  Historically filesystem drivers are NOT capable of handling
> such things, as evidenced by the large number of bugs that tools such as
> fsfuzzer tend to trigger.  If you want to use IMA as-designed then you
> need to perform a relatively extensive audit of filesystem and fsck code.

Seems to me exploitable bugs in fs code should be fixed regardless of EVM...

> Furthermore, even when the filesystem does not have any security issues
> itself, you are assuming that intentionally malicious data-aliasing
> between "trusted" and "untrusted" files can have no potential security
> implications.  You should look at the prevalence of simple stupid "/tmp"
> symlink attacks for more counter-examples there.
> 
> In addition, IMA relies on the underlying attribute and data caching
> properties of the VFS, which won't hold true for intentionally malicious
> corrupted filesystems.  It effectively assumes that writing data or
> metadata for one file will not invalidate the cached data or metadata for
> another which is blatantly false when filesystem extents overlap each
> other.

As discussed, this is a tradeoff in IMA-Appraisal-HASH, which allows
data in protected files to be updated. The attacks do not work on EVM,
IMA-Appraisal-DigitalSignature, or IMA attestation, where the attacker
cannot forge valid verifiers, even with arbitrary access to the raw device.

> Overall, the IMA architecture assumes that if it loads and validates the
> file data or metadata that it cannot be changed except through a kernel
> access to that particular inode.  For a corrupted filesystem that is
> absolutely untrue.
 
IMA-Appraisal-HASH does as a deliberate tradeoff. The other integrity 
modules do not.

thanks
dave


  reply	other threads:[~2011-07-14 15:07 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-29 19:50 [PATCH v7 00/16] EVM Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 01/16] security: new security_inode_init_security API adds function callback Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 02/16] integrity: move ima inode integrity data management Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 03/16] xattr: define vfs_getxattr_alloc and vfs_xattr_cmp Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 04/16] evm: re-release Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 05/16] evm: add support for different security.evm data types Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 06/16] security: imbed evm calls in security hooks Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 07/16] evm: evm_inode_post_removexattr Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 08/16] evm: imbed evm_inode_post_setattr Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 09/16] evm: add evm_inode_init_security to initialize new files Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 10/16] evm: call evm_inode_init_security from security_inode_init_security Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 11/16] evm: crypto hash replaced by shash Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 12/16] evm: additional parameter to pass integrity cache entry 'iint' Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 13/16] evm: evm_verify_hmac must not return INTEGRITY_UNKNOWN Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 14/16] evm: replace hmac_status with evm_status Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 15/16] evm: permit only valid security.evm xattrs to be updated Mimi Zohar
2011-06-29 19:50 ` [PATCH v7 16/16] evm: add evm_inode_setattr to prevent updating an invalid security.evm Mimi Zohar
2011-06-29 20:53 ` [PATCH v7 00/16] EVM Kyle Moffett
2011-06-29 20:53   ` Kyle Moffett
2011-06-29 23:42   ` Mimi Zohar
2011-06-30  1:57     ` Kyle Moffett
2011-06-30  1:57       ` Kyle Moffett
2011-06-30  3:51       ` Mimi Zohar
2011-06-30 22:32         ` Kyle Moffett
2011-06-30 22:32           ` Kyle Moffett
2011-07-14 15:07           ` David Safford [this message]
     [not found]         ` <BANLkTin-x1kkXiowUYjBS_tr4iwDrzNQkA@mail.gmail.com>
2011-07-01 14:34           ` Mimi Zohar
2011-07-01 21:55             ` Mimi Zohar
2011-07-14 15:07       ` David Safford
2011-07-14 15:07         ` David Safford
2011-07-18 13:45         ` Serge E. Hallyn
2011-07-14 15:07   ` David Safford
2011-06-30 21:06 ` Ryan Ware
2011-06-30 21:06   ` Ryan Ware
2011-06-30 22:37   ` Mimi Zohar
2011-06-30 22:37     ` Mimi Zohar
2011-07-01  2:02     ` Ware, Ryan R
2011-07-01  2:02       ` Ware, Ryan R
2011-07-18 23:52 ` James Morris
2011-07-19 20:56   ` Mimi Zohar
2011-08-09  1:53     ` James Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1310656066.3845.244.camel@localhost \
    --to=safford@watson.ibm.com \
    --cc=jmorris@namei.org \
    --cc=kyle@moffetthome.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.